-
Notifications
You must be signed in to change notification settings - Fork 20
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
khepri_machine: Restore #unregister_projection{}
for backward-compatibility
#286
khepri_machine: Restore #unregister_projection{}
for backward-compatibility
#286
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #286 +/- ##
==========================================
+ Coverage 89.55% 89.66% +0.10%
==========================================
Files 21 21
Lines 3122 3124 +2
==========================================
+ Hits 2796 2801 +5
+ Misses 326 323 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codewise 👍
For the commit description and the comment in apply/3
though I think it's more general to say that the command might be "in existing Raft logs". The WAL is a temporary place to store commands as a performance optimization and the command might also be in segment files. But both are part of the Raft log.
bf2bfc3
to
a068e78
Compare
Good point @the-mikedavis, I improved the wording. |
…tibility [Why] Even though this version of Khepri never emits `#unregister_projection{}`, existing Ra log files may still contain that command. Therefore, it may still be applied after an upgrade of Khepri. [How] To not break these existing deployment, `apply/3` accepts `#unregister_projection{}` again. It converts it to the new `#unregister_projections{}` command and recurses.
a068e78
to
e45323e
Compare
I added a test case that just makes sure the machine code won't crash. It doesn't verify if the projections are actually unregistered. |
Why
Even though this version of Khepri never emits
#unregister_projection{}
, existing WAL files may still contain that command.How
To not break these existing deployment,
apply/3
accepts#unregister_projection{}
again. It converts it to the new#unregister_projections{}
command and recurses.