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

Emit the new local_query tuple only if query options are set #451

Merged

Conversation

dumbbell
Copy link
Member

@dumbbell dumbbell commented Jul 1, 2024

Why

Using the new tuple isn't compatible with older versions of Ra. This breaks local/leader queries if there are Ra members running an old version.

How

We don't need the new {local_query, QueryFun, Options} tuple if there is no options. We can use the old {local_query, QueryFun} in this case and keep the compatibility.

@dumbbell dumbbell added the bug label Jul 1, 2024
@dumbbell dumbbell requested a review from kjnilsson July 1, 2024 14:01
@dumbbell dumbbell self-assigned this Jul 1, 2024
@dumbbell dumbbell marked this pull request as ready for review July 8, 2024 10:14
[Why]
Using the new tuple isn't compatible with older versions of Ra. This
breaks local/leader queries if there are Ra members running an old
version.

[How]
We don't need the new `{local_query, QueryFun, Options}` tuple if there
is no options. We can use the old `{local_query, QueryFun}` in this case
and keep the compatibility.
@dumbbell dumbbell force-pushed the emit-new-local_query-tuple-only-if-options-are-set branch from 5c4c204 to 26b0259 Compare July 8, 2024 10:45
@michaelklishin michaelklishin added this to the 2.12.1 milestone Jul 8, 2024
@michaelklishin
Copy link
Member

I've set the milestone to 2.12.1 assuming this does not require a minor bump.

@michaelklishin michaelklishin merged commit a5d8e3b into main Jul 8, 2024
10 checks passed
@michaelklishin michaelklishin deleted the emit-new-local_query-tuple-only-if-options-are-set branch July 8, 2024 13:01
@dumbbell
Copy link
Member Author

dumbbell commented Jul 8, 2024

That's correct, I don't think it requires a minor bump.

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

Successfully merging this pull request may close these issues.

3 participants