Using pyramid.paster.bootstrap as context manager #1822

[marioidival]

No description provided.

[tseaver]

I don't understand the motivation here: the environment currently returned from bootstrap has a 'closer' key, but it won't be called when the with block exists (which is the only reason I can think of to add it).

[davisagli]

I agree -- I proposed something like this in IRC a week or two ago, but I was intending that the context manager would automatically call the closer on exit.

[marioidival]

Sorry, then I guess I did not understand the description of the issue. There speaks the bootstrap function should be context manager. I left it like that. prepare the function that makes the "closer", right?

[davisagli]

The reason for wanting it to be a context manager is so that I don't have to remember to call the closer myself.

[tseaver]

Not to mention that wrapping it in a @contextmanager is backward-incompatible: it breaks normal imperative usage, because the thing returned when not used in a with statement is no longer a dictionary.

If we want to enable such a feature, it would be more sensible to add a new function, wrapped in @contextmanager, which yielded the result of calling bootstrap() and then called the closer. E.g.:

@contextmanager
def boostrap_cm(config_uri, request=None, options=None):
    """Self-closing boostrapped environment.

    Usable only via ``with bootstrap_cm(...):``.
    """
    env = bootstrap(configu_uri. request, options)
    yield env
    env['closer']()

[mmerickel]

I assume this work is in reference to #1822 which is not intended to break compatibility.

The returned env should subclass a dict and provide __enter__ and __exit__ functions to behave as a context manager.

[marioidival]

@mmerickel @tseaver @davisagli

class EnvManager(dict):
    def __init__(self, env):
        self.update(**env)
    def __enter__(self):
        return self
    def __exit__(self, type, value, tb):
        self["closer"]()

# nothing change in prepare

# pyramid/paster.py

from pyramid.scripting import prepare, EnvManager

def bootstrap(...)
    app = get_app(config_uri, options=options)
    env = prepare(request)
    env['app'] = app
    return EnvManager(env)

[marioidival]

    def test_it_request_with_registry_as_context_manager(self):
        request = DummyRequest({})
        request.registry = dummy_registry
        result = self._callFUT('/foo/bar/myapp.ini', request)
        with result:
            self.assertEqual(result['app'], self.app)
            self.assertEqual(result['root'], self.root)
            self.assertTrue('closer' in result)

    def test_it_request_with_registry_is_not_context_manager(self):
        request = DummyRequest({})
        request.registry = dummy_registry
        result = self._callFUT('/foo/bar/myapp.ini', request)
        self.assertEqual(result['app'], self.app)
        self.assertEqual(result['root'], self.root)
        self.assertTrue('closer' in result)

root@c7ac8588abbe:/azk/pyramid# nosetests pyramid.tests.test_paster.Test_bootstrap
..
----------------------------------------------------------------------
Ran 2 tests in 0.341s

OK

[mmerickel]

I'm not sure that calling self.update is the proper way to subclass a dict. I think you shouldn't even have an __init__.

[mmerickel]

You will want to update your test to assert that the closer was called afterward.

[marioidival]

Ok @mmerickel :)

[mmerickel]

Also the idea is that prepare itself will return the EnvManager, and then bootstrap will just automatically work (add a test just to be sure). But most of the tests should be on prepare and not bootstrap.

[marioidival]

Nice, copy that.

[mcdonc]

Sorry to chime in late, and I'm even sorrier to add more hoops to jump through to be able to merge this.

I'd merge this if:

• The test_scripting.py file passed a pyflakes check (it fails currently).
• __getattr__ is removed from EnvManager unless there's a good reason for it.
  It's not a good enough reason to want to type self.closer() in __exit__ instead
  of self['closer']().
• The test_scripting.py file contained a test that asserted that the closer is
  called when an exception happens within the with: block.
• The test scripting.py file contained a test that asserted that the context manager
  usage does not swallow exceptions.
• The "Scripting" chapter documentation was updated to explain the use of
  the return value of prepare as a context manager.

[marioidival]

@mcdonc @tseaver @mmerickel @davisagli
Guys, I apologize, I think I can not complete this PR. I'm having some problems in my home and not getting to have time to finish it.

[mmerickel]

Hey @marioidival thanks for the update and your efforts on the patch. If you'd like to submit another commit with your name in CONTRIBUTORS.txt we can continue your work, otherwise we'll have to close this.

In the meantime I'm going to close this PR.

[marioidival]

Hello guys, I'm back, I'll continue this PR, all right??

[stevepiercy]

@marioidival glad to see you back! Please proceed and submit a new commit to this PR, then one of us will reopen the issue. Thank you!