Feature: support trilogy adapter #607
Conversation
and console script
and specify #cpk_mysql_subquery if TrilogyAdapter is defined
One thing we need to ensure overall is that and ActiveRecord::Result is returned as the guts of the gem is looking to work with that object. A Trilogy::Result was being returned as `value`and that doesn't respond to #empty?
We need to load some more files as AR isn't aware of trilogy
lib/composite_primary_keys/connection_adapters/abstract/database_statements.rb
Outdated
Show resolved
Hide resolved
|
Thanks for working on this! Had a chance to review the code, and overall looks pretty good. I did leave some comments however if you could take a look. |
|
FYI I enabled all tests for this MR - right now they all fail due to requiring trilogy. |
there's a pattern already in place as the constant may not always be defined
|
Progress - now tests pass again except the Trilogy ones. Looks like a problem loading trilogy:
|
I accidentally added it to its own github action and we for sure need trilogy here
|
@cfis pushed up some commit that address the 
|
|
Still failing, with a slightly different error now:
|
so load it and make the DB connection
Sorry for the confusion. I mentioned in my previous comment that I need to ensure AR is aware of Trilogy. I didn't commit that change as I know you mentioned you wanted it removed before. I've gone ahead and added the last commit with the changes I mentioned earlier. Do you mind kick off another round of the CI? |
|
And onto the next issue:
|
|
Thoughts? |
Apologies for the delay. Let me look take a look and see. |
.github/workflows/trilogy.yml
Outdated
| mysql -h 127.0.0.1 -e "CREATE USER 'test'@'localhost' IDENTIFIED WITH mysql_native_password BY 'test';" -u${{ env.MYSQL_USER }} -p${{ env.MYSQL_PASSWORD }} | ||
| mysql -h 127.0.0.1 -e "GRANT ALL PRIVILEGES ON *.* TO 'test'@'localhost' WITH GRANT OPTION;" -u${{ env.MYSQL_USER }} -p${{ env.MYSQL_PASSWORD }} | ||
| mysql -h 127.0.0.1 -e "FLUSH PRIVILEGES;" -u${{ env.MYSQL_USER }} -p${{ env.MYSQL_PASSWORD }} |
There was a problem hiding this comment.
@cfis do you know if there's a way for me to run the GitHub actions on my own fork? I'm making educated guesses at the Github action issues around these mysql commands.
There was a problem hiding this comment.
I think via the .github/workflows/trilogy.yml file. But since that is part of the PR maybe it doesn't work (might have to be merged first?)
There was a problem hiding this comment.
@cfis I can spin up a new PR and see if merging in other components first resolve this issue.
There was a problem hiding this comment.
@cfis any objections in me breaking this into two PR's?
There was a problem hiding this comment.
Works for me. Sorry this taking so long to merge. I can try and focus on it next week.
There was a problem hiding this comment.
First PR is here: #610
There was a problem hiding this comment.
I see two more PRs after that one. The first with the trilogy guts and the second being the GitHub action. How do you feel about that?
zmariscal
force-pushed
the
feature--support-trilogy-adapter
branch
from
f74ebbc to
f79e913
Compare
|
@cfis I've go ahead and removed the Github Action for now. You should be able to run the |
|
Sorry for the delay on this and thanks for keeping at it! |
Purpose
There's a new DB adapter called trilogy and this gem should be aware of it.
Implementation
Make the gem aware of the trilogy adapter by adding the gem and subsequent requires. We need to make sure the database statements
valueis actually andActiveRecord::Resultand not aTrilogy::Resultwhen the adapter istrilogy. One of the reasons we need to care about this is becauseTrilogy::Resultdoesn't have#empty?implemented.