Remove the EnvFactory class from the CTS code#4107
Open
clumens wants to merge 10 commits intoClusterLabs:mainfrom
Open
Remove the EnvFactory class from the CTS code#4107clumens wants to merge 10 commits intoClusterLabs:mainfrom
clumens wants to merge 10 commits intoClusterLabs:mainfrom
Conversation
It was always provided, so remove the option for it to be None. Ref T680
It's not an Environment instance, even though it acts an awful lot like one. It's actually a CtsLab instance. Ref T680
Nothing uses it. Ref T680
The Corosync class is only used by cts-attrd, cts-exec, and cts-fencing which are all standalone programs. Nothing in the wider cts python module uses it. Thus we can make this change without having to modify anything anywhere else. Ref T680
nrwahl2
requested changes
May 7, 2026
| # Create the Cluster Manager object. | ||
| # Currently Corosync2 is the only available cluster manager. | ||
| cm = Corosync2() | ||
| cm = Corosync2(env) |
Contributor
There was a problem hiding this comment.
The Corosync2 constructor needs to take this and use it instead of creating its own instance.
Contributor
Author
There was a problem hiding this comment.
Github seems to be on a quest to make comments harder to see in the context of the code they're commenting on, so I might have missed something. But it's already doing that. A Corosync2 is a ClusterManager, and its __init__ method takes the same env instance and sets that as an attribute that gets used everywhere.
Nothing uses this yet, which will cause a pylint warning, but the patch to make use of it is going to be pretty large so I wanted to split this out separately. Ref T680
Instead of instantiating it through the EnvFactory class, pass through the one that we got in test_list. This also allows eliminating a ton of pylint warnings. Ref T680
Instead of using the EnvFactory class, we can just instantiate an Environment and pass it around everywhere. We still want there to only be one instance of this class first because it does a lot of argument processing which takes time, and second because each instance will get its own random number. Ref T680
And with this, there are no more factory classes in our python code. Fixes T680
These don't do anything besides wrap functions in the logging module.
This file contains hidden or 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
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.
This is it.