-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Loader Contexts #58853
Loader Contexts #58853
Conversation
ef25e7b
to
91493e6
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 new requirement should be added to requirements/static/pkg
since it will become a hard dependency, in fact, it should actually be added to requirements/base.txt
since its a hard dependency and applies to all platforms.
dbef631
to
5715dde
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.
There are still a lot of print debug statements, but you should know that already.
Left some questions.
76e3a76
to
ee93d39
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.
Approving based on others', and that I know this fixes #54045.
What does this PR do?
Adds loader contexts so that multiple loaders can't stomp on each other.
What issues does this PR fix or reference?
Prior to these changes salt's 'magic attributes' (
__utils__
,__salt__
,__opts__
, ect..). Would be added as global attributes to a s loaded module's namespace. This had the un-intended side effect of allowing a method call from one loader to change something in another loader. This can be particularly problematic for__context__
and__opts__
. This can also cause problems for loaders added with unique configs.There is also an addition of a new
pack_self
attribute to Salt'sLazyLoader
class. This allows the loader to expose itself as a magic dunder without creating circular references on in the loader's pack dictionary. Those circular references are the cause of some memory leaks. Thepack_self
argument should be used to avoid memory leaks.Previous Behavior
Due to the global nature of Salt's dunder methods, calls from one loader could interfere with other loaders.
New Behavior
Loaders use contextvars to provide a context that is specific the the loader that loads the function being called.
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.