Skip to content
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

Port #51499 to master #55398

Merged
merged 1 commit into from
Dec 5, 2019
Merged

Port #51499 to master #55398

merged 1 commit into from
Dec 5, 2019

Conversation

oeuftete
Copy link
Contributor

@oeuftete oeuftete commented Nov 21, 2019

TODOs:

  • figure out how to port this or if it's required. Things have changed somewhat,

What does this PR do?

Ports #51499 to master. See that PR for additional information.

It looks like the original loader changes are no longer necessary with refactoring that has occurred since the original issue was addressed. The original author's tests look like they will show whether that is the case.

Commits signed with GPG?

No.

@oeuftete oeuftete requested a review from a team as a code owner November 21, 2019 20:47
@ghost ghost requested a review from waynew November 21, 2019 20:47
@sagetherage
Copy link
Contributor

this looks like it just needs the WIP title removed and all other checks pass - but also needs a rebase, is that right @waynew ?

@sagetherage
Copy link
Contributor

@mattp- @oeuftete this looks like it needs to be rebased and WIP in the title removed.

when lazyLoader was getting instantiated from inside already lazyloaded
modules (ie state), pillar/grain (and probably other) globals were
already populated. In this case they weren't getting dereferenced
properly when wrapped via the NamespacedDict wrapper, causing an
infinite loop. This should fix that scenario.

Fixes saltstack#50655 (comment)
@oeuftete oeuftete changed the title WiP: Port #51499 to master Port #51499 to master Dec 4, 2019
@oeuftete oeuftete force-pushed the port-51499-to-master branch from 4fb06f8 to 2202fed Compare December 4, 2019 22:00
@oeuftete
Copy link
Contributor Author

oeuftete commented Dec 4, 2019

I've rebased. Note that the original change to the main code base was removed, since it looked not applicable with refactors to the loader since. The added tests have been left, though, and appeared to be passing (before this rebase, anyway.)

@oeuftete
Copy link
Contributor Author

oeuftete commented Dec 4, 2019

@Akm0d It looks like you might be able to assess whether no change in the loader here is appropriate, based on #54878 (comment)?

@dwoz dwoz merged commit 4f321b4 into saltstack:master Dec 5, 2019
@oeuftete oeuftete deleted the port-51499-to-master branch December 5, 2019 22:41
@oeuftete oeuftete restored the port-51499-to-master branch June 26, 2020 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants