clear the sqlalchemy session at the beginning of each api handler call

[dannygoldstein]

re-opening fixing https://github.com/cesium-ml/baselayer/pull/212 to address this issue:

Ari 4:38 PM @danny it looks like the Session.remove() addition was actually included in 211, so the merge commit for 212 is empty: 6de3248

[pep8speaks]

Hello @dannygoldstein! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file app/handlers/base.py:

Line 162:49: W291 trailing whitespace

Comment last updated at 2021-03-23 04:51:32 UTC

[stefanv]

Probably cleaner to close the session after app initialization, so that handler does the expected: create new scope at beginning, destroy scope at end.

[dannygoldstein]

OK I'll move this logic to skyportal app initialization - though it will still be an issue for any other apps that use baselayer

On Mon, Mar 22, 2021 at 5:02 PM Stefan van der Walt < @.***> wrote:

Probably cleaner to close the session after app initialization, so that handler does the expected: create new scope at beginning, destroy scope at end.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cesium-ml/baselayer/pull/213#issuecomment-804481288, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVEFYDMMXJ4TITICIE6J5TTE7LCRANCNFSM4ZUC7YRA .

-- Danny Goldstein http://astro.caltech.edu/~danny

[dannygoldstein]

@stefanv Actually, a bit confused here - handlers don't currently create new scope in prepare -- that's what this PR does. If we want to create new scope each time a handler is called, why not call remove or rollback at the beginning of each handler call?

[dannygoldstein]

The scoped_session.remove() method first calls Session.close() on the current Session, which has the effect of releasing any connection/transactional resources owned by the Session first, then discarding the Session itself. “Releasing” here means that connections are returned to their connection pool and any transactional state is rolled back, ultimately using the rollback() method of the underlying DBAPI connection.

At this point, the scoped_session object is “empty”, and will create a new Session when called again. As illustrated below, this is not the same Session we had before:

[dannygoldstein]

Actually, there may be a happy medium using session.expired...

[acrellin]

@dannygoldstein why not just clear the session after app initialization?

[dannygoldstein]

@dannygoldstein why not just clear the session after app initialization?

@acrellin There is no make_app in baselayer

[acrellin]

@dannygoldstein yeah, so we just do this in the parent app (and include a note in BL about it)

[dannygoldstein]

OK, I'm cool with that.

[dannygoldstein]

superseded by https://github.com/skyportal/skyportal/pull/1801

[dannygoldstein]

It turns out that both clearing the session at the start of every request and clearing the session once after app startup leave the application in a thread-unsafe state for the reasons discussed in https://github.com/cesium-ml/baselayer/pull/214. Closing in favor of that PR.