pyeve / eve-sqlalchemy

SQLAlchemy data layer for Eve-powered RESTful APIs
http://eve-sqlalchemy.readthedocs.io
Other
232 stars 70 forks source link

SQL object creates a blank local flask_sqlalchemy.SQLAlchemy object. #20

Closed edthedev closed 9 years ago

edthedev commented 9 years ago

Is there a particular reason that the eve_sqlalchemy.SQL object initializes with an internally created instance of flask_sqlalchemy.SQLAlchemy, rather than taking one as a constructor option?

https://github.com/RedTurtle/eve-sqlalchemy/blob/master/eve_sqlalchemy/__init__.py#L27

db = flask_sqlalchemy.SQLAlchemy()

https://github.com/RedTurtle/eve-sqlalchemy/blob/master/eve_sqlalchemy/__init__.py#L63

def init_app(self, app):

It seems like the class would be much simpler to use if the constructor was able to take a previously built instance of flask_sqlalchemy.SQLAlchemy.

def init_app(self, app, db=None):

Psedocode example that is currently impossible:

# Make exactly the instance of flask_sqlalchemy.SQLAlchemy needed.
flask_app = app()
flask_sql = flask_sqlalchemy.SQLAlchemy(**kwargs)
eve_sql = eve_sqlalchemy.SQL(app=flask_app, db=flask_sql)

This change would seem to allow some of the currently enforced retroactive injection to be replaced by declaration and combination, resulting in more readable client code.

I could be missing some critical nuance of the architecture here, of course.

amleczko commented 9 years ago

This is somehow released to #16. I would leave the discussion there. Please let me know if you think those two issues should be kept separably.