flask-mongorest
flask-mongorest copied to clipboard
support for flask app factory pattern
settled the conflicts I found after fetching #55 @andviro
closes #81
ping ;)
The errors from CircleCI are just pyflakes. There's quite a few for * imports. Do you guys want to update the flake8 config or fix them?
Hey @frankV, thanks for championing this :) Overall it looks good aside from a few minor issues I commented on in the code (though @thomasst should also take a look). As for pyflakes, I see that all of the issues happen in the example
code. We should probably move to named imports in there instead of ignoring more rules. That said, the errors only manifested in this PR because its build was triggered with an upgraded flake8 version. I'm fine with opening a separate issue for that and ignoring F405 here.
@wojcikstefan fixed up the minor things and I'll finish up the others tomorrow. I'll wait for @thomasst's remarks before issuing another PR.
didn't forget about this, getting back to it so for now I'm going to reissue the PR with the changes I implemented... let's go from there
@wojcikstefan I got it all in with the exception of
- didn't skip the "ugly hasattr conditions" in init but I can with a little more time
- updated the formatting to my example solution on init line 87
Just wanted to ping you guys one more time on this, let me know if you want something changed.
Let's get this merged in! 👍
Hi @frankV, I've been on vacation for the last 2 weeks - will look at it soon! Thank you so much for following up on this one!
I just reviewed this code and read up on init_app
.
While this PR allows lazy initialization, it still doesn't conform to the guidelines. See http://flask.pocoo.org/docs/0.11/extensiondev/:
Note on init_app As you noticed, init_app does not assign app to self. This is intentional! Class based Flask extensions must only store the application on the object when the application was passed to the constructor. This tells the extension: I am not interested in using multiple applications.
When the extension needs to find the current application and it does not have a reference to it, it must either use the current_app context local or change the API in a way that you can pass the application explicitly.
That being said, I'm not sure if there's a good solution to make sure the URL map is applied to all the apps, in case init_app
is called multiple times with different apps. Thoughts?
I'm not sure I can understand the use case, but that may be my limited experience with using multiple apps.
All I can think to do is investigate how similar extensions handle this, then test it out in my fork.
Hey, can someone please merge this pull request. I need this functionality in one of my projects
Lazy initialization is a must on any flask extension, will this ever be merged? 😢
Ohh god 3 years and init_app
still not added, why!!
Sorry! I just stumbled across this and didn't realize a PR was pending here for so long. I've just merged delayed init_app
support with https://github.com/closeio/flask-mongorest/pull/137