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

push threadlocals while executing config.include functions #2989

Merged
merged 3 commits into from
Apr 2, 2017

Conversation

mmerickel
Copy link
Member

We don't push threadlocals in the main because there is no clear contract on its lifecycle. However we can push them during config.include(...) calls so that they are available there. Just a little more incentive to put all your code into include functions!

@mmerickel mmerickel added this to the 1.9 milestone Apr 2, 2017
@mmerickel mmerickel merged commit b536bd9 into Pylons:master Apr 2, 2017
@piotr-dobrogost
Copy link

We don't push threadlocals in the main because there is no clear contract on its lifecycle.

Michael, could you please elaborate on this lifecycle issue (I'm curious)?
Btw, is there any section in the docs for this kind of information?

@mmerickel
Copy link
Member Author

mmerickel commented Apr 3, 2017

The lifecycle issue is that there is no clear ending point to the Configurator lifetime. This means that it cannot automatically push the threadlocals because it cannot remove them at an appropriate spot. The only possible spot is make_wsgi_app but that is not good enough because the Configurator can be used to make multiple wsgi apps in testing scenarios and such.

If you combine this PR with #2873 and the pending #2874 we can rewrite example code as below and have the entire config-time covered by threadlocals.

with Configurator(settings=settings) as config:
    config.include(...)
    config.add_view(...)
    return config.make_wsgi_app()

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 this pull request may close these issues.

3 participants