Avoid mutable types as default args. #373
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
This change removes two places where we use a set as a default value. Mutable containers should not be used as default arguments as they are created when the function is defined (basically, the first time the module is imported) and then that one instance is used anytime the method is invoked without that argument specified. This means that changes to the container will persist to the next time the function is invoked which could lead to bugs down the road. I do not believe we're currently experiencing any bugs due to this as we never seem to mutate the containers. Instead, we use the data to make new sets inside of the functions.
abadger
force-pushed
the
remove-mutable-default
branch
from
1802466
to
2e796b0
Compare
bocekm
approved these changes
Nov 30, 2021
Venefilyn
approved these changes
Nov 30, 2021
Merged
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 change removes two places where we use a set as a default value.
Mutable containers should not be used as default arguments as they are
created when the function is defined (basically, the first time the
module is imported) and then that one instance is used anytime the
method is invoked without that argument specified. This means that
changes to the container will persist to the next time the function is
invoked.
This change seems to fix a bug we're seeing on oracle7 where we assemblea list of packages to downgrade that includes gcc & gcc-c++ but then we find
that systemd is a protected package. At that point, the list of
packages to downgrade seems to revert to just systemd and we build up
the list again. This time, when we see that gcc is going to be
downgraded but gcc-c++ requires it, we do not decide to downgrade
gcc-c++. I haven't tracked down precisely why this is but I suspect that
gcc-c++ is getting placed in the default set for the
excluded_pkgsargument so that when we iterate over the pakcages the second time we
never re-add gcc-c++ to the list of pakcages to downgrade.