-
Notifications
You must be signed in to change notification settings - Fork 221
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
6853 upgrade flux #7439
6853 upgrade flux #7439
Conversation
eb208c0
to
6d3db5a
Compare
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.
This looks mostly like the right direction. The unavailable paramManager values worry me. Can we get access to them (in tests) by waiting longer?
250599a
to
4276860
Compare
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.
The direction looks good. I'm still concerned about the missing initial update. Plz let me know what you figure out.
031418b
to
ed9b636
Compare
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 have skimmed this a little. If you need me to take a closer look, LMK. Otherwise, I defer to @Chris-Hibbert
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.
Thanks for the work-arounds. They resolve my concerns.
LGTM
ed9b636
to
3e5b4c5
Compare
closes: #7248
closes: #6853
Description
This makes fluxAggregator upgradable.
To do so it also has to solve #7248. They're in separate commits. I kept them in this PR because fluxAggregator upgrade demonstrates the changes sufficing.
Security Considerations
Scaling Considerations
Documentation Considerations
Testing Considerations