Split the freeze toggles and checkers #610
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In some cases, the toggle should be turned to or checked against a runtime value. With separate methods for on/off values, it requires `if-else` expressions or statements. It could be much more simple if there is only one method to turning it to any state, and one method for waiting, and one method for checking. Especially in the light of coming changes when the freeze toggles are split to toggles & checkers, and there could be more than one toggle per checker — having too many methods is confusing and prone to errors. However, the on/off checks are left as is — to keep the code readability, and to avoid confusion when checking for `bool(toggle_set)`, where the "toggle set" is a collection of toggles: a collection is typically true when it contains any items, but we need a special logic over these items.
|
This pull request introduces 1 alert when merging 39329d2 into 8601875 - view on LGTM.com new alerts:
|
nolar
force-pushed
the
peering-freeze-toggles-checkers
branch
from
39329d2
to
1cec5a1
Compare
nolar
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In preparation for the coming changes when there are multiple peering objects from multiple namespaces monitored for a single operator, it is crucial that the freeze-checkers and freeze-toggle are split and managed separately.
Previously, it was a single flag, which was either on or off.
Now, it is multiple flags, each is in its own state, and one checker, which is off when all toggles are off, or on when at least one toggle is on — i.e. if there is at least one peering/namespace with a conflicting operator.
However, in the current codebase, only one freeze flag is possible, as the operator can handle one and only one namespace (or be cluster-wide); multiple namespaces are not yet supported.
The change is essentially an extracted preparation for a bigger PR to make it smaller and more readable.
Relies on #608 and #609.