ext:database backend regression after upgrade to 1.10.0

[marcinkuzminski]

After an upgrade to 1.10.0 from 1.9.1 we're getting an exception when using a DB backend for sessions.

Traceback (most recent call last):
  File "/nix/store/l5a8jq5hflr83kfl71xk3wpfppbf8y2y-python2.7-pyramid-1.9.2/lib/python2.7/site-packages/pyramid/router.py", line 276, in default_execution_policy
    return router.invoke_request(request)
  File "/nix/store/l5a8jq5hflr83kfl71xk3wpfppbf8y2y-python2.7-pyramid-1.9.2/lib/python2.7/site-packages/pyramid/router.py", line 252, in invoke_request
    request._process_response_callbacks(response)
  File "/nix/store/l5a8jq5hflr83kfl71xk3wpfppbf8y2y-python2.7-pyramid-1.9.2/lib/python2.7/site-packages/pyramid/request.py", line 92, in _process_response_callbacks
    callback(self, response)
  File "/nix/store/ngd4zqqk2hjvfhx1wi9djiajhivc993r-python2.7-pyramid-beaker-0.8/lib/python2.7/site-packages/pyramid_beaker/__init__.py", line 30, in session_callback
    self.persist()
  File "/nix/store/f2jdwcf6818assnxf7qbpyn6lsgj9pc8-python2.7-beaker-1.10.0/lib/python2.7/site-packages/beaker/session.py", line 832, in persist
    self._session().save()
  File "/nix/store/f2jdwcf6818assnxf7qbpyn6lsgj9pc8-python2.7-beaker-1.10.0/lib/python2.7/site-packages/beaker/session.py", line 467, in save
    **self.namespace_args)
  File "/nix/store/f2jdwcf6818assnxf7qbpyn6lsgj9pc8-python2.7-beaker-1.10.0/lib/python2.7/site-packages/beaker/ext/database.py", line 71, in __init__
    url = url or sa_opts['sa.url']
KeyError: 'sa.url'

Here's the session configuration we use:

beaker.session.type = ext:database
beaker.session.table_name = db_session
beaker.session.sa.url = mysql://some:credentials@127.0.0.1/rhodecode
beaker.session.sa.pool_recycle = 3600
beaker.session.sa.echo = false

[amol-]

Previously it was actually an unexpected behaviour that any unsupported option for the DatabaseNamespaceManager would just end up being proxied as SQLAlchemy option.

That lead as a side effect that options like sa.url which was unsupported by the DatabaseNamespaceManager ended up being proxied to SQLAlchemy and as they were prefixed exactly like the sqlalchemy option prefix they would replace the sqlalchemy options.

This was mostly by chance and is no longer supported because backends are now expected to just ignore options that they do not support. Which allows to better handle backend specific options in each backend.

The options for the DatabaseNamespaceManager were originally expected as a dictionary in a sa_opts option::

    ``sa_opts``
            A dictionary of SQLAlchemy keyword options to initialize the engine
            with.

I realise that this is less convenient as parsing of dict like options through an .ini file is not viable, but we need a way to distinguish between sqlalchemy options and unsupported options and a more explicit way of providing sqlalchemy options instead of relying on what is mostly an undocumented internal detail.

[marcinkuzminski]

Thanks for the explanation, I understand it was an undocumented feature, however i think this change will break a lot of applications, and furthermore, there's no alternative to use except to code it in the python implementations.

This was mostly by chance and is no longer supported because backends are now expected to just ignore options that they do not support. Which allows to better handle backend specific options in each backend.

So what does that fix, or what problem does it solve? For me the previous behavior was a good extension mechanism.

Like for us (RhodeCode) when we ship a bundled application it heavily uses beaker caching, and we have various settings based on customer requirements (mostly to handle mysql problems when using DB backends). It requires a lot of effort for us to turn it from .ini settings into python dict settings, and I believe this will also happen to others.

• I know it's not documented, but if you look online/google/stackoverflow there's lots of examples to use .sa.* options just because of requirements to set the MySQL pool_recycle option. This change makes usage of Beaker with ext:database option to not work correctly in many cases unless you do some actual coding, and pass along the engine options down to sqlalchemy.

How about making only the ext:database backend use the .sa.* options while others don't? Or introduce maybe some reserved option to pass along values via .ini file, like pyramid_dogpile does.

dogpile_cache.REGION.arguments.*

This makes it more explicit.