No need to accept multi_result as conn option in Ruby binding
#68
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adrianna-chang-shopify
force-pushed
the
ac-multi-results-cleanup
branch
from
41134be to
cb027f9
Compare
paarthmadan
reviewed
Apr 20, 2023
We added the ability to pass `:multi_result` as a connection option to the client in the Ruby binding, but the MULTI_RESULTS flag is set by default so this isn't necessary.
adrianna-chang-shopify
force-pushed
the
ac-multi-results-cleanup
branch
from
cb027f9 to
9c22cb6
Compare
eileencodes
approved these changes
Apr 20, 2023
composerinteralia
added a commit
that referenced
this pull request
Apr 26, 2023
#57 set multi-results as a default. To safely upgrade to the latest version of Trilogy, we want to turn that off for now, and then turn it on one cluster at a time. This commit removes the default from the C library and into the Ruby C extension so we can disable it as needed. (We had an option to enable multi results, but removed it in #68 because the option was always a no-op. This PR adds the option back, with some modification).
luanzeba
added a commit
that referenced
this pull request
May 17, 2023
#57 set multi-results capability as a default. To safely upgrade to the latest version of Trilogy, we want to turn that option off and then turn it on one cluster at a time. This commit removes the multi-results capability default from TRILOGY_CAPABILITIES_CLIENT but will still set it unless the client is initialized with `multi_results: false`. We had an option to enable multi results, but removed it in #68 because the option was always a no-op. This PR adds the option back, with some modification. Co-authored-by: Daniel Colson <composerinteralia@github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up to #57
We added the ability to pass
:multi_resultas a connection option to the client in the Ruby binding, but theMULTI_RESULTSflag is set by default so this isn't necessary. (I made the changes to the Ruby binding before we decided to set the flag as part of the default bitmask).Might as well clean this code up in the C-ext :)