Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

with_trilogy_connection: materialize_transactions method is overshadowed by the keyword argument. #60

Closed
dhruvCW opened this issue Aug 14, 2023 · 2 comments · Fixed by #61

Comments

@dhruvCW
Copy link
Contributor

dhruvCW commented Aug 14, 2023

#59 introduced a regression wherein it renamed a keyword parameter.

- materialize_transactions if uses_transactions
+ materialize_transactions if materialize_transactions

The problem with this is that materialize_transactions is a function, but in the context of this method it is evaluated as the keyword argument. Thus the above statement became a no-op.

@lorint
Copy link
Collaborator

lorint commented Aug 14, 2023

Aha! Now I am intrigued, @dhruvCW :)
In this we are hoping to approach the naming found in edge Rails. I will look into this in earnest.

Hope that the other thing is coming together as well -- trilogy-libraries/trilogy/#98.

Thank you dearly for helping to make Trilogy great!

@dhruvCW dhruvCW changed the title raw_execute: use_transaction regression with_trilogy_connection: materialize_transactions method is overshadowed by the keyword argument. Aug 15, 2023
@dhruvCW
Copy link
Contributor Author

dhruvCW commented Aug 15, 2023

Aha! Now I am intrigued, @dhruvCW :) In this we are hoping to approach the naming found in edge Rails. I will look into this in earnest.

Hope that the other thing is coming together as well -- trilogy-libraries/trilogy/#98.

Thank you dearly for helping to make Trilogy great!

Hey 👋

I was incorrect in my original hypothesis. Turns out the issue was the call to materialize_transaction. Given the with_trilogy_connection is a replacement of a rails component the PR introduced an inconsistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants