Support additional SQLAlchemy parameters.

[yangbaepark]

I found some SQLAlchemy parameters are helpful when we have unreliable network connectivity between applications and database.

• creator helps to create custom DBAPI connections.
• echo_pool will log all check-ins/outs of the pool.
• pool_pre_ping tests connections for linveness upon each checkout.
• pool_reset_on_return allows to override default behavior upon return.

By default, all those options are turned off (or following the default behavior in SQLAlchemy) for backward compatibility.

For more information about the parameters, please refer to the following official SQLAlchemy documentation:
http://docs.sqlalchemy.org/en/latest/core/engines.html#engine-creation-api

[gilbsgilbs]

I'm also willing to see those options in flask-sqlalchemy. They would need to be documented though.

[redixin]

Some of these options are available only in sqlalchemy 1.2.0b3 (pre release)

[gilbsgilbs]

@redixin Yes, but it doesn't break support for previous versions anyways. It should just be documented.

[redixin]

@gilbsgilbs
Need to update sqlalchemy first
https://github.com/mitsuhiko/flask-sqlalchemy/blob/master/setup.py#L34

[gilbsgilbs]

@redixin I don't see your point, could you elaborate? This selector is compatible with sqlalchemy 1.2+.

[johanols]

Looking closed and open PRs it seems that this is a highly requested feature. I for one would love to be able to set pool_pre_ping without having to sub-class SQLAlchemy. Reading through the comments, it assess these are the still open issues with this PR:

• The added parameters needs to be documented.
• The version pinning of the sqlalchemy package needs to be adjusted.

Is there a possibility to get this fixed and merged within the scope of this PR?

[redixin]

@gilbsgilbs

@redixin I don't see your point, could you elaborate? This selector is compatible with sqlalchemy 1.2+.

Version 1.2 wasn't released at that moment. It is released now, but we still have 'SQLAlchemy>=0.8.0' in setup.py

Need to update setup.py as well.

[johanols]

@gilbsgilbs Thank you for your quick respond!

So making this PR merge ready would be really simple. Unfortunately, the original author doesn't seem to be very active. Would it in that case make sense to abandon this PR in favor of a new one?

I could arrange that if desired.

[yangbaepark]

Hello Olsson and Gilbert, I am sorry for the late response. I will update PR. Regards, Yangbae From: Johan Olsson <notifications@github.com> Sent: Thursday, March 7, 2019 12:18 AM To: pallets/flask-sqlalchemy <flask-sqlalchemy@noreply.github.com> Cc: Park, Yangbae <yangbae@amazon.com>; Author <author@noreply.github.com> Subject: Re: [pallets/flask-sqlalchemy] Support additional SQLAlchemy parameters. (#540) @gilbsgilbs<https://github.com/gilbsgilbs> Thank you for your quick respond! So making this PR merge ready would be really simple. Unfortunately, the original author doesn't seem to be very active. Would it in that case make sense to abandon this PR in favor of a new one? I could arrange that if desired. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#540 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AehLn9LxgkKt0Wax3rvtS4qonxHco-zkks5vUMs4gaJpZM4PZoao>.

[davidism]

It doesn't look like you need to update anything right now, I'm not sure why users have commented saying you need to do so.

[gilbsgilbs]

Version 1.2 wasn't released at that moment. It is released now, but we still have 'SQLAlchemy>=0.8.0' in setup.py

Changing this selector would be a breaking change. There is no reason to prevent people from using this library with SQLAlchemy 1.1 and downwards, the library will keep working as before. We just need to document that the POOL_PRE_PING option will only work starting from SQLAlchemy 1.2+ (and document all the options that are added in this PR).

[davidism]

Just a heads up though, this probably won't get merged as I'd rather support a more general way to pass arguments to the pool, engine, and session rather than adding individual config keys. I'm leaving it open mainly because we'll need to organize all the related PRs and issues at some point.

Thanks for your work though, I don't mean to say it's not appreciated, just that it points to a larger goal we have.

[yangbaepark]

Just a heads up though, this probably won't get merged as I'd rather support a more general way to pass arguments to the pool, engine, and session rather than adding individual config keys. I'm leaving it open mainly because we'll need to organize all the related PRs and issues at some point.

Thanks for your work though, I don't mean to say it's not appreciated, just that it points to a larger goal we have.

Sure, that makes sense. I agree it won't be sustainable to keep adding keys whenever SQLAlchemy introduces new parameters.

[rsyring]

@davidism is this approach sufficient for you?

#166 (comment)

If so, a PR should be trivial.

[davidism]

Sure, that could work, I'd like it to pull from a class/instance init attribute of defaults as well so it's not only configurable via config (:thinking:).

[rsyring]

@davidism I can do that.

[rsyring]

Thank you for your contribution and sorry this took so long. We opted to go in a different direction but it will solve your use case. Please see: #684