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

new mongomock commit breaks flask init #220

Closed
ntcong opened this issue Mar 28, 2016 · 8 comments
Closed

new mongomock commit breaks flask init #220

ntcong opened this issue Mar 28, 2016 · 8 comments
Labels
type: bug Something isn't working
Milestone

Comments

@ntcong
Copy link

ntcong commented Mar 28, 2016

This small code snippet won't work

from flask import Flask
from flask.ext.mongoengine import MongoEngine

app = Flask(__name__)
db = MongoEngine(app)
current_app.config['TESTING'] == True and....
> RuntimeError: working outside of application context

I think because on _create_connection we don't have a app context yet. Init_app should take an app and get config from it, instead of using local app context.
Adding a parameter to _create_connection should be enough so I can do a PR if it's acceptable.

@ntcong ntcong changed the title New mongomock: working outside of application context new mongomock commit breaks flask init Mar 28, 2016
@ntcong
Copy link
Author

ntcong commented May 23, 2016

any update on this ?
There are two ways to solve this: either we pass flask's app instance to _create_connection, or we pass the "TESTING" config (or remove it).
I don't know what is "good" by standard of the project so I need some input so I can make a PR

@lafrech lafrech added the type: bug Something isn't working label May 23, 2016
@lafrech
Copy link
Member

lafrech commented May 30, 2016

I stumbled upon this today as well.

AFAIU, @ntcong's diagnosis is correct.

Removing this line fixes it:

current_app.config['TESTING'] == True and

@losintikfos, can you explain the need to test for this? I mean, if the host starts with mongomock, should we care about TESTING being in the settings?

If there is any reason to test this, then init_app may pass app to _create_connection.

@losintikfos, I know you're working on the connection part and you don't have much time for it. Should we

  • send a PR with the removal of the app.config['TESTING'] test?
  • send a PR with app being passed to _create_connection?
  • wait until you're done with the whole thing?

Thanks.

@lafrech lafrech added this to the 0.8 milestone May 30, 2016
@losintikfos
Copy link
Member

losintikfos commented Jun 1, 2016

@lafrech I will ensure TESTING is used appropriately in the _create_connection enhancement and throughout the app context.

Thanks.

@lafrech
Copy link
Member

lafrech commented Jun 10, 2016

Hi @losintikfos. Congratulations for your work on the connection module.

I don't know if you're done with it or not, but just in case, I thought I'd let you know I just tried latest master version of flask-mongoengine on my project and I still have this issue.

It fails here:

is_test = current_app.config.get('TESTING', False)

@losintikfos
Copy link
Member

@lafrech can you send me a test case to simulate here please.

Ta!

@ntcong
Copy link
Author

ntcong commented Jun 10, 2016

I think my original test still didn't work

from flask import Flask
from flask.ext.mongoengine import MongoEngine

app = Flask(__name__)
db = MongoEngine(app)

It's still the same problem: you shouldn't use current_app when initialize an extension. The prefered way is passing an app instance

@losintikfos
Copy link
Member

@ntcong I having a look into this.

Ta

@lafrech
Copy link
Member

lafrech commented Jun 10, 2016

I trimmed down my use case to this:

from flask import Flask
from flask_mongoengine import MongoEngine


class Config(object):
    SECRET_KEY = ("whatever")

    MONGODB_SETTINGS = {
        'db': 'db',
        'host': 'localhost',
        'port': 27017,
        'username': 'user',
        'password': 'pwd'
    }


me = MongoEngine()


def create_app(**kwargs):

    app = Flask(__name__, **kwargs)

    app.config.from_object(Config)

    me.init_app(app)

    return app

app = create_app()

I think this use case (app factory) is less common.

However, @ntcong's example is precisely the use case described in the docs, so I'm wondering which use case actually work.

Anyway, thanks @losintikfos for investigating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants