flask-mongorest icon indicating copy to clipboard operation
flask-mongorest copied to clipboard

support for flask app factory pattern

Open frankV opened this issue 8 years ago • 13 comments

settled the conflicts I found after fetching #55 @andviro

closes #81

frankV avatar Jul 03 '16 17:07 frankV

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?

frankV avatar Jul 07 '16 15:07 frankV

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 avatar Jul 09 '16 00:07 wojcikstefan

@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.

frankV avatar Jul 09 '16 04:07 frankV

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

frankV avatar Aug 24 '16 21:08 frankV

@wojcikstefan I got it all in with the exception of

  1. didn't skip the "ugly hasattr conditions" in init but I can with a little more time
  2. updated the formatting to my example solution on init line 87

frankV avatar Aug 24 '16 22:08 frankV

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! 👍

frankV avatar Sep 04 '16 20:09 frankV

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!

wojcikstefan avatar Sep 07 '16 17:09 wojcikstefan

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?

thomasst avatar Sep 26 '16 20:09 thomasst

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.

frankV avatar Sep 29 '16 19:09 frankV

Hey, can someone please merge this pull request. I need this functionality in one of my projects

sriramsv avatar May 29 '17 05:05 sriramsv

Lazy initialization is a must on any flask extension, will this ever be merged? 😢

richin13 avatar Aug 08 '18 17:08 richin13

Ohh god 3 years and init_app still not added, why!!

adnanmuttaleb avatar Aug 07 '19 12:08 adnanmuttaleb

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

AlecRosenbaum avatar Dec 21 '21 14:12 AlecRosenbaum