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

Allow engine options to be set through the constructor like session options. #487

Closed
wants to merge 1 commit into from

Conversation

keithjstone
Copy link

Package does not allow setting of custom engine options without monkey patching or inheriting the base class. This would allow options to be set and remembered prior to creating the engine in a blessed manner.

Our specific use-case is setting use_native_uuids to False on a postgres connection but I think this is more generally useful.

app.config['SQLALCHEMY_BINDS'] = {
'foo': 'sqlite://'
}
db = fsa.SQLAlchemy(app, engine_options=dict(echo=True))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not the best example for a unit test considering this one can be set through the flask config (SQLALCHEMY_ECHO)

@rsyring
Copy link
Contributor

rsyring commented Jun 23, 2017

For what it's worth, I think the approach I documented in this comment is a better approach that this PR as it permits the engine options to be set in the Flask configuration.

The suggestion:

            options = {'convert_unicode': True}
            self._sa.apply_pool_defaults(self._app, options)
            self._sa.apply_driver_hacks(self._app, info, options)
            if echo:
                options['echo'] = echo

            # next two lines are new
            config_engine_opts = self._app.config.get('SQLALCHEMY_ENGINE_OPTS', {})
            options.update(config_engine_opts)

            self._engine = rv = sqlalchemy.create_engine(info, **options)

And it's only two lines difference.

@asteinlein
Copy link

👍 to having this in Flask configuration.

@rsyring
Copy link
Contributor

rsyring commented Mar 8, 2019

Thank you for your contribution and sorry this took so long. I believe #684 will solve this use case.

@rsyring rsyring closed this Mar 8, 2019
@keithjstone
Copy link
Author

Huge thanks for the fix! We had found a workaround locally so we could continue to use the published versions. Will be looking to update!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants