-
Notifications
You must be signed in to change notification settings - Fork 150
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
Implement support for CRaC #2250
base: main
Are you sure you want to change the base?
Conversation
- Update the FinalClassTransformer to close the .new and .old files. - Implement the CRaC API on Log4jLogger to shutdown logging on checkpoint and restart it on restore.
…t into crac_testing
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2250 +/- ##
============================================
- Coverage 70.57% 70.53% -0.04%
Complexity 10017 10017
============================================
Files 842 844 +2
Lines 40619 40560 -59
Branches 6163 6146 -17
============================================
- Hits 28668 28611 -57
+ Misses 9167 9166 -1
+ Partials 2784 2783 -1 ☔ 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.
I feel funny having the supportability metric in the http client wrapper. It could as well be in the Logger, but it is a responsibility of neither.
Almost feel like we should have a CracService or util that registers itself with Crac for the callbacks, and our listeners would then register on this.
But this is just another abstraction/complexity layer just to keep up with the single responsibility principle.
So I won't hold the review because of this. But I think the other things are worth a look.
Several adjustments made: