rdegges / flask-dynamo

DynamoDB integration for Flask.
http://flask-dynamo.readthedocs.org/en/latest/
The Unlicense
141 stars 36 forks source link

Dynamo within application context before and after using tables property #16

Closed tcco closed 7 years ago

tcco commented 8 years ago

Discovered a state issue with flask-dynamo when running unit test(pytest and unitest) on my flask application. Initially the error was no table was named %s when flask-dynamo attempted to query so I checked the application context state.

from flask import _app_ctx_stack as stack cox = stack.top print dir(ctx)

This was in hopes of looking for keys such as dynamo_tables, dynamo_connection, etc; however none of these keys were there. Along with the missing keys, dynamo.app was nil as well. However in the unit test code setUp, dynamo.app was still set and able to create tables.

Our hack fix was to call flask.current_app, set that as dynamo.app, and than call dynamo.tables, which recovered the state due to the check for variables in context when retrieving this property. Is this something that should be set earlier?

Some other flask packages like sqlalchemy and security add the to flask.app.extensions to store specific states. Maybe this would help to store dynamo in context?

This is the text referring to flask.app.extensions (https://github.com/pallets/flask/blob/67e391921c50f02190be8407d4a02efe8d2d28f6/flask/app.py)

: a place where extensions can store application specific state. For

    #: example this is where an extension could store database engines and
    #: similar things.  For backwards compatibility extensions should register
    #: themselves like this::
    #:
    #:      if not hasattr(app, 'extensions'):
    #:          app.extensions = {}
    #:      app.extensions['extensionname'] = SomeObject()
    #:
    #: The key must match the name of the extension module. For example in
    #: case of a "Flask-Foo" extension in `flask_foo`, the key would be
    #: `'foo'`.
    #:
    #: .. versionadded:: 0.7
    self.extensions = {} 
rdegges commented 8 years ago

Ohhh. This is quite interesting. I do believe you are correct.

I did not realize this until just now with this report. I suppose a good solution would be to re-write the internals to use app.extensions then? I'd be happy to do that =)

tcco commented 8 years ago

Flask does seem to do some behind-the-scenes magic with app.extensions so I couldn't guarantee it is the fix but I could reasonably assume it is a step in the right direction. I've seen states classes that'll store tuples of information like the app, connections, and such.

Looking at class _SQLAlchemyState

Am happy to test any given changes! Let me know how else I can help as well.