coleifer / flask-peewee

flask integration for peewee, including admin, authentication, rest api and more
http://flask-peewee.readthedocs.org/
MIT License
777 stars 179 forks source link

Support deferring configuration of database to after initialization #69

Open xiaq opened 11 years ago

xiaq commented 11 years ago

Peewee allows you to do this, by passing None to __init__:

from peewee import *
db = SqliteDatabase(None)
db.init('a.db')

Flask-SQLAlchemy too, by leaving out the app argument:

db = SQLAlchemy()
db.init_app(app)

This is important if you need to create the app in a function instead of in the module global, which makes unit testing easier and is better practice. The complete form for the above example is:

db = SQLAlchemy()

def create_app():
    app = Flask(__name__)
    db.init_app(app)
    return app

Deferring configuration of database still allows you to put db in the module global and use db.Model as base class for ORM models.

xiaq commented 11 years ago

I found out the author is precisely that of Peewee, so I needn't have explained this much. :-)

I also identified a problem. A deferred Peewee database still needs to have its type (sqlite/postgre/...) specified. Seems Peewee needs to be enhanced so that a deferred peewee.Database can has its type unknown.

emosenkis commented 11 years ago

+1. My unit tests run twice as long because they are making a connection to the MySQL database for every simulated request. Using a config that points to an in-memory SQLite database doubles the speed, but it would probably still be much faster if it could be told not to connect until later. Here's the MySQL query log created by running unit tests:

Time                 Id Command    Argument
130728 21:45:13   146 Connect   schdl@localhost on schdl
          146 Query set autocommit=0
          146 Quit  
          147 Connect   schdl@localhost on schdl
          147 Query set autocommit=0
          147 Quit  
          148 Connect   schdl@localhost on schdl
          148 Query set autocommit=0
          148 Quit  
          149 Connect   schdl@localhost on schdl
          149 Query set autocommit=0
          149 Quit  
130728 21:45:14   150 Connect   schdl@localhost on schdl
          150 Query set autocommit=0
          150 Quit  
          151 Connect   schdl@localhost on schdl
          151 Query set autocommit=0
          151 Quit  
          152 Connect   schdl@localhost on schdl
          152 Query set autocommit=0
          152 Quit  
          153 Connect   schdl@localhost on schdl
          153 Query set autocommit=0
          153 Quit  
          154 Connect   schdl@localhost on schdl
          154 Query set autocommit=0
          154 Quit  
          155 Connect   schdl@localhost on schdl
          155 Query set autocommit=0
          155 Quit  
          156 Connect   schdl@localhost on schdl
          156 Query set autocommit=0
          156 Quit  
          157 Connect   schdl@localhost on schdl
          157 Query set autocommit=0
          157 Quit  
          158 Connect   schdl@localhost on schdl
          158 Query set autocommit=0
          158 Quit  
          159 Connect   schdl@localhost on schdl
          159 Query set autocommit=0
          159 Quit
dmr commented 11 years ago

What is the status of this feature? Are there any forks which implemented this? Is there a reason why flask-peewee handles is differently?

coleifer commented 11 years ago

I need to fix this. Will try and get things cleaned up in the next day or two.

mangrish commented 11 years ago

+1. What's the workaround that people are doing when they use application factories? global instance?

xiaq commented 11 years ago

@mangrish Build a proxy class. A bit clumsy, though.

xiaq commented 11 years ago

@coleifer Seems this has been implemented with f387f73? It has been some time since I used flask-peewee so I'm not quite sure - have been using raw peewee lately...

It would be nice if the deferred configuration of engine is implemented in the peewee proper though :)

xiaq commented 11 years ago

Ah... not quite there yet. You still need to specify the app when initializing the database:

import flask, flask_peewee
app = flask.Flask(__name__)
db = flask_peewee.Database(app)

better make it possible to write this, a la Flask-SQLAlchemy:

import flask, flask_peewee

db = flask_peewee.Database()

def create_app():
    app = flask.Flask(__name__)
    db.init_app(app)
    return db

I'll probably send a PR tonight (UTC+8 here :)...

xiaq commented 11 years ago

Ah, harder than I though...

flask_peewee.Database needs to have its Model determined before init_app is called. Which is not possible without determining the database engine...

So better implement deferred engine selection in peewee... :)

arnuschky commented 11 years ago

Crossposting: I think that this issue is related to https://github.com/coleifer/flask-peewee/issues/103

It would be great if we could find a way to do configuration independent of a given app, and independent of the usage of flask at all (i.e., only use the peewee models in a separate non-web app).

EDIT: I just realized that github does the pingback automagically for you, thus sorry for the noise.

lmas commented 10 years ago

Anyone found a better, "clean" solution yet?

arnuschky commented 10 years ago

No. Would be interested in this as well, same use-case as mentioned by @xiaq this time.

Not sure if this can be solved without substantial changes in peewee.

arnuschky commented 10 years ago

So, I found a (hacky) solution for this. In my app, I use a init_app method as proposed by @xiaq. All my models extend peewee.Model. The code below, when placed in the init_app method, will scan the package for all classes that extend peewee.Model. If it finds such a class, it will monkey-patch the newly created database object into the Model's meta info.

    for _, name, _ in pkgutil.iter_modules(package_path):
        m = importlib.import_module('%s.%s' % (package_name, name))
        for item in dir(m):
            item = getattr(m, item)
            if item.__class__ == Model.__class__:
                item._meta.database = self.database

Code needs of the package path and name, eg., the package with all your models.

Not pretty, but it works. This allows us also to get around get_model_class and some other things from Database()

coleifer commented 10 years ago

Have you guys tried using the peewee Proxy object? That will probably work. Maybe I will fiinally fix this...

arnuschky commented 10 years ago

Works like a charm. Much nicer than my hack (but I was soo proud! ;) ) Thanks a bunch @coleifer

kolanos commented 10 years ago

Is there an example somewhere on how to use the Proxy object with flask-peewee? flask-peewee checks to see if the engine is a subclass of Database, which Proxy is not. It also requires a name parameter which it will attempt to pass to Proxy during initialization. Do I need to override flask-peewee 's Database class?