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

make the configurator a context manager #2872

Closed
mmerickel opened this issue Dec 15, 2016 · 1 comment · Fixed by #2874
Closed

make the configurator a context manager #2872

mmerickel opened this issue Dec 15, 2016 · 1 comment · Fixed by #2874
Milestone

Comments

@mmerickel
Copy link
Member

There's some subtle bugs in apps that hit certain APIs during configuration because parts of pyramid rely on threadlocal state that may or may not be available.

An example:

config = Configurator()
config.override_asset(to_override='myapp:locale/', override_with='yourapp:locale/')
config.add_translation_dirs('myapp:locale')

The result here is that the translations are still using myapp:locale because add_translation_dirs resolves those asset paths at config-time. Asset overrides work by using custom pkg_resources hooks that only affect pkg_resources when a registry is pushed on the threadlocal stack. Normally this would require surrounding relevant code with config.begin() and config.end(). We've done a poor job of documenting this.

I propose an optional context manager that we can implement easily (we already did something like this for p.paster.bootstrap in #2760) in order to manage the lifecycle.

with Configurator() as config:
    config.override_asset(to_override='myapp:locale/', override_with='yourapp:locale/')
    config.add_translation_dirs('myapp:locale')

related: #2760 #2046

@mmerickel
Copy link
Member Author

The particular example here was fixed by #2873 for the record.

mmerickel added a commit to mmerickel/pyramid that referenced this issue Apr 3, 2017
mmerickel added a commit to mmerickel/pyramid that referenced this issue Apr 3, 2017
@mmerickel mmerickel added this to the 1.9 milestone Apr 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant