vytas7 / falcon-sqla Goto Github PK

Write a short tutorial, and Aggregate docstrings into reference docs.

As communicated by @CaselIT , automatically choosing a write engine if the session is currently flushing (automatically enabled by default, see SessionOptions.write_engine_if_flushing) is apparently not bullet proof (as explained in sqlalchemy/sqlalchemy#5407 (comment) ).

This was implemented along the lines of a DIY recipe from SQLAlchemy docs. Revise what the options are. If we fail to improve this to cover the majority of common scenarios, it might be worth changing the default to False to not give a false sense of safety.

In any case, falcon-sqla, if configured, is normally choosing read/write engines according to the HTTP method for the Falcon request in question, and write_engine_if_flushing is provided just as an edge case fallback, so the impact of this bug should be somewhat limited.

See also: falconry/falcon#658

The default logic, commit on success, rollback on failure is a sensible one

maybe it could be useful to add the ability of a resource to customize this logic.
I'm not sure how to best implement it. Probably a function in the resource class that returns what to do (commit, rollback, nothing) is the best solution.

Split off from #7.

Write a short tutorial on how this could be used with a Falcon application in order to reduce the amount of boilerplate code.

The test suite, although hitting 100% branch coverage, is a bit chaotic at the time of writing.
I've cleaned up DB fixtures in conftest.py, so they're probably good enough as-is.

Clean up the tests, add more cases logically testing typical usage rather than just hitting all lines.

falcon-sqla wraps streams using ClosingStreamWrapper.

This class does not have a read attribute, and it causes falcon to not use the wsgi_file_wrapper
https://github.com/falconry/falcon/blob/2556abce9afd9e0ea55d6aada09c64c311ca688e/falcon/api.py#L832

This can lead to large performance regressions when the stream is a file, since it can get iterated over one byte at the time.
See Pylons/waitress#291

We should support falcon v3

Currently when read_from_rw_engines or write_to_rw_engines are False and no engine is read-only or write-only all the engine are used.

This behavior is quite strange, since the option is not respected.

Raising an error in this case would probably be more appropriate, the problem with the exception is that by adding the engines one by one it's easy to encounter an error while adding them.

I'm not sure how to best support this scenario.
One option is to make the engine selection lazy, so that only on the first use we check for this, the user does not have an immediate feedback with this approach