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

Fix fileserver backends global overwritten by _pillar #22942

Closed
wants to merge 1 commit into from
Closed

Fix fileserver backends global overwritten by _pillar #22942

wants to merge 1 commit into from

Conversation

bersace
Copy link
Contributor

@bersace bersace commented Apr 22, 2015

Refs #22941

@thatch45
Copy link
Contributor

Woah! This will make all masters run 20 times slower!
Lets take a closer look at where the underlying issue is

@thatch45 thatch45 added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Apr 22, 2015
@cachedout
Copy link
Contributor

Agreed with @thatch45 here. This will cause a massive performance hit to the master. I am going to close this PR in favour of continuing this discussing in #22941. Thanks, @bersace

@cachedout cachedout closed this Apr 22, 2015
@bersace
Copy link
Contributor Author

bersace commented Apr 23, 2015

Hi,

I also feel uncomfortable with this solution. But at least, it's a starting point to fix this :)

The ground of the issue is : fileserver backends are singleton with __opts__ pushed in the context of the module. Pillar monkey patch the backends by reloading file client with pillar=True. So we must a least restore.

The real solution would be to refactor fileserver backends as class with a context. I think self.opts is enough for a context ;-) But for now, the fix can be to just clean the monkey patching after pillar are loaded.

This remember the ages of guerilla patching in Zope ;-)

Thanks for reviewing this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged Tests-Passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants