-
Notifications
You must be signed in to change notification settings - Fork 368
refactor: adding options to allow all without specifying all of the members in whitelisting for node swapping feature flags #7650
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
Conversation
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 pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):
-
Update
unreleased_changelog.md(if there are behavior changes, even if they are
non-breaking). -
Are there BREAKING changes?
-
Is a data migration needed?
-
Security review?
How to Satisfy This Automatic Review
-
Go to the bottom of the pull request page.
-
Look for where it says this bot is requesting changes.
-
Click the three dots to the right.
-
Select "Dismiss review".
-
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:
-
Done.
-
$REASON_WHY_NO_NEED. E.g. for
unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".
Brief Guide to "Externally Visible" Changes
"Externally visible behavior change" is very often due to some NEW canister API.
Changes to EXISTING APIs are more likely to be "breaking".
If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.
If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.
Reference(s)
For a more comprehensive checklist, see here.
GOVERNANCE_CHECKLIST_REMINDER_DEDUP
No breaking changes
The PR refactors how empty lists are interpreted for the node swapping feature.
Previously an explicit statement of who is allowed to swap the nodes and where was needed. While this approach is good and it works fine, there is an issue that we have a lot of subnets and a lot of node operators:
Having to state all of the subnets and all of the node operators will make the
flags.rsreally unreadable. Furthermore, having all of them stated doesn't bring any value.NOTE: it will still be possible to disallow the feature on specific subnets or for specific node operators. We will just have to make explicit lists without the said subnets or node operators.
In the future, we may look into turning whitelisting model into a blacklisting one if we see that we have to disallow feature for specific node operators or for specific subnets.