New Session fixing attempt

[TomNaessens]

No description provided.

[TomNaessens]

New attempt to fix sessions in the Flask app. The Flask app still runs, no idea if it actually updates fine (if the new changes are immediately present in the app, will have to test this on production).

Stuff I am absolutely not sure about (next to all ranker and non-Flask stuff):
database.py does not create a Session() anymore. Only scoped_session creates one, and before every request (in the before_request flask callback), a Session() is created.
So I don't know what to do with these:
https://github.com/ZeusWPI/aichallenge/pull/85/files#diff-8b9c2f1c191b122b39e95c50dfb12ce2L40 Should a scoped_session with wrapper be used here?
https://github.com/ZeusWPI/aichallenge/pull/85/files#diff-1b067c12eda3233d670b7251051cb0edL8 Same for this, no idea

[TomNaessens]

Should we use a with scoped_session() as db: here? If not, why?

[wschella]

No, we shouldn't need them on queries since those should not cause errors that require a session-rollback.

[TomNaessens]

That's correct, but at that point, there isn't an open session as Session() is only called in 2 places: before every Flask request handling and at the start of a scoped transaction. So wrapping that bit in a scoped_session block would open a session, and rollback if the query errors (which the ORM should ignore as there are no open transacation).

[Procrat]

The commit/rollback part would indeed be unnecessary, so maybe we can make two kinds of context managers? Or maybe change Session.query(...) to Session().query(...)?

[TomNaessens]

That last wouldnt close that session connection then?
On 8 Apr 2016 11:00, "Stijn Seghers" notifications@github.com wrote:

In battlebots/database/access.py
#85 (comment):

@@ -37,7 +37,8 @@ def remove_bot(user, botname):
% (user.nickname, botname, code_dir))
pass

• bot = session.query(Bot).filter_by(user=user, name=botname).one_or_none()

The commit/rollback part would indeed be unnecessary, so maybe we can make
two kinds of context managers? Or maybe change Session.query(...) to
Session().query(...)?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/ZeusWPI/aichallenge/pull/85/files/1478b56574b04b5a61ef6433eb7d796aa0f7b628#r58997827

[wschella]

Opening a session for each request certainly seems the way to go, as people keep recommending it. It's definitely the most scalable and least bugprone way to do it. We should attempt this and not call it quits before we manage to do it.

[Procrat]

@silox: I just reread your first comments. Why wouldn't there be an open session? If we chose (2), than either the ranker or the web server should have opened one. Otherwise this code shouldn't be called in the first place. Right?

[wschella]

@Procrat The ranker and webserver don't share their sessions, as they both use different enginges. But you are right that there should a session should be available because the Session object is connected to a session-factory.

[Procrat]

Exactly.

[TomNaessens]

@Procrat Yes! Good thinking, that should indeed be the case and fix this
problem
On 8 Apr 2016 11:26, "Stijn Seghers" notifications@github.com wrote:

In battlebots/database/access.py
#85 (comment):

@@ -37,7 +37,8 @@ def remove_bot(user, botname):
% (user.nickname, botname, code_dir))
pass

• bot = session.query(Bot).filter_by(user=user, name=botname).one_or_none()

@silox https://github.com/Silox: I just reread your first comments. Why
wouldn't there be an open session? If we chose (2), than either the ranker
or the web server should have opened one. Otherwise this code shouldn't be
called in the first place. Right?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/ZeusWPI/aichallenge/pull/85/files/1478b56574b04b5a61ef6433eb7d796aa0f7b628#r59000580