make SA engine configuration more flexible

[rsyring]

fixes #166 - make it possible to pass additional options to create_engine()
fixes #671, #681 - deprecation warnings from SA 1.3

[rsyring]

Current Travis failures:

• Python 3.3 fails due to tox supporting 3.4+
• Python 2.6 fails due to SQLAlchemy needing Py 2.7+
• docs_html: /home/travis/build/pallets/flask-sqlalchemy/.tox/docs_html/lib/python3.6/site-packages/flask_sqlalchemy/init.py:docstring of flask_sqlalchemy.SignallingSession.get_bind:63:Unknown
  interpreted text role "paramref".

@davidism I'm not sure where FSA sits in terms of Python version support. Created an issue to discuss that: #685

The docs error I can look into but I figured I'd see if you knew what the problem is. This PR doesn't change that file, so I think the problem existed previously.

[rsyring]

When #687 is approved and merged, I'll rebase this PR on that branch and that should resolved all the CI issues.

[rsyring]

I need to review how/why get_engine() caches the engine before this is merged.

[rsyring]

I need to review how/why get_engine() caches the engine before this is merged.

I'd like to punt on that discussion and have opened #698 for discussion.

@davidism I've rebased this PR, Travis is passing, and I believe the commits in this PR are distinct enough to not squash. Let me know if you would like me to change anything and/or if I'm good to merge.

[rsyring]

Code updated to address two review requests. The lingering one with respect to mock usage remains. If the status quo is acceptable, then this is ready for merge and I'll prep a 2.4 release for @davidism's review.

[rsyring]

The doc tests are failing in CI with:

/home/travis/build/pallets/flask-sqlalchemy/docs/changelog.rst.rst:23:broken link: https://github.com/pallets/flask-sqlalchemy/pull/556 (403 Client Error: Forbidden for url: https://github.com/pallets/flask-sqlalchemy/pull/556)

ERROR: InvocationError for command '/home/travis/build/pallets/flask-sqlalchemy/.tox/docs/bin/sphinx-build -W -b linkcheck -d /home/travis/build/pallets/flask-sqlalchemy/.tox/docs/tmp/doctrees docs docs/_build/linkcheck' (exited with code 2)

That tox run passes when I test it locally, maybe GitHub is having some issues. Anyway, I don't think that failure should hold up the merge of this PR. I'll kick off the job a couple more times to see if I can get it to pass.

Definitely think GitHub is having issues, another CI run gives a different PR number for the failure:

/home/travis/build/pallets/flask-sqlalchemy/docs/changelog.rst.rst:20:broken link: https://github.com/pallets/flask-sqlalchemy/pull/551 (403 Client Error: Forbidden for url: https://github.com/pallets/flask-sqlalchemy/pull/551)

[davidism]

Are there any separate features we can deprecate and roll into this? If so, I'd like those deprecation warnings to be in the next release.

[aydumoulin]

@davidism The two deprecations that arose from fast forwarding sqlalchemy to 1.3+ in my postgres flavor usage were:

• SADeprecationWarning: The create_engine.convert_unicode parameter and corresponding dialect-level parameters are deprecated, and will be removed in a future release. Modern DBAPIs support Python Unicode natively and this parameter is unnecessary
  default.DefaultDialect.__init__(self, **kwargs)
• SADeprecationWarning: Use .persist_selectable
  info = getattr(mapper.mapped_table, 'info', {})

[davidism]

That's not what I'm referring to, those are deprecations in SQLAlchemy. I'm referring to features that we want to deprecate.

[woodb]

With the recent publishing of security vulnerabilities in SQLAlchemy v1.2.17 through v1.3.0b2 (CVE-2019-7164) getting pushed out to folks via GitHub alerts, this issue is becoming a bigger deal for users that want to upgrade to v1.3.x.

With these security issues abound, I would like to suggest that we consider expediting getting this merged and released (i.e. without waiting for more stuff to put into the next release)

I haven't read the code here, so not sure if a patch version is appropriate, but really would like to see this fix get out so that my test suite can have useful output again, hah.

If there's anything that still needs to be done on this PR to get #671 and #681 closed, please let us know. Thanks!

[rsyring]

From @davidism regarding his comment on deprecations above:

was specifically talking about engine settings in that comment
Did any individual config options get added before that could be rolled up? I think ECHO is the only one to keep since it's good for debugging. http://flask-sqlalchemy.pocoo.org/2.3/config/
There are mostly pool-related config keys.
And NATVIE_UNICODE was no longer needed, right?
So we might want to issue deprecation warnings if they're set, and add their value to ENGINE_OPTIONS for now.

I'll be doing that work shortly and then prepping a release.

[fuhrysteve]

@woodb just to be clear to any other concerned observers of this repo, there are no known security vulnerabilities related to CVE-2019-7164 in any currently supported major versions of sqlalchemy as long as you are escaping user input in places in the docs that have clearly stated for many years they are intentionally not escaped.

[swuecho]

Thanks! especially for SQLALCHEMY_ENGINE_OPTIONS.

[billyfung]

ditto^
engine options works great for psycopg2 use_batch_mode=True