Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docs: adds section about DB session management #124

Merged
merged 2 commits into from
Nov 20, 2019
Merged

docs: adds section about DB session management #124

merged 2 commits into from
Nov 20, 2019

Conversation

roksys
Copy link
Contributor

@roksys roksys commented Nov 18, 2019

@roksys roksys added this to Backlog in Sprint Week 46-47 (2019) - V3.2 Release via automation Nov 18, 2019
@roksys roksys moved this from Backlog to Pending review in Sprint Week 46-47 (2019) - V3.2 Release Nov 18, 2019
@equadon equadon self-assigned this Nov 19, 2019
equadon
equadon reviewed Nov 19, 2019
View reviewed changes
docs/sessions_management.rst Outdated

Transactions are tied to requests
---------------------------------
In Invenio, a transaction is tied to a HTTP request. By default, the transaction is
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In Invenio, a transaction is tied to a HTTP request. By default, the transaction is
In Invenio, a transaction is tied to an HTTP request. By default, the transaction is

docs/sessions_management.rst Outdated
on the records table when the database transaction isolation level is below repeatable read
isolation level.

When are SQL statements sent to the database server.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When are SQL statements sent to the database server.
When are SQL statements sent to the database server?

docs/sessions_management.rst Outdated
rolled back, and afterwards the entire transaction will be rolled back unless
the exception is caught.

For instance in the following code example, the entire transaction will be rolled backed:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For instance in the following code example, the entire transaction will be rolled backed:
For instance in the following code example, the entire transaction will be rolled back:

docs/sessions_management.rst Outdated
raise Exception()
db.session.commit()

On the other, in the following example, the propagation of the exception is stopped,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other.. what? On the other hand?

@equadon equadon removed their assignment Nov 19, 2019
@mvidalgarcia mvidalgarcia self-assigned this Nov 20, 2019
mvidalgarcia
mvidalgarcia reviewed Nov 20, 2019
View reviewed changes
docs/index.rst Outdated
@@ -20,6 +20,7 @@ Invenio-DB.
configuration
usage
alembic
sessions_management
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about session_management?

docs/sessions_management.rst Outdated
In Invenio, a transaction is tied to an HTTP request. By default, the transaction is
automatically rolled back unless you explicitly call ``db.session.commit()``.

Exceptions causes rollback
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be either "Exception causes rollback" or "Exceptions cause rollback".

docs/sessions_management.rst Outdated
server when needed, or when explicitly requested via e.g. ``db.session.flush()`` or
``db.session.commit()``.

This means that in many cases, SQL insert and update statements are not sent to the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This means that in many cases, SQL insert and update statements are not sent to the
This means that in many cases, SQL INSERT and UPDATE statements are not sent to the

docs/sessions_management.rst Outdated
What about nested transactions?
-------------------------------
Nested transactions are using database save points, which allow you to do a partial
rollback. Also, nested transactions causes a flush to the database, meaning that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
rollback. Also, nested transactions causes a flush to the database, meaning that
rollback. Also, nested transactions cause a flush to the database, meaning that

docs/sessions_management.rst Outdated
db.session.commit()

On the other hand, in the following example, the propagation of the exception is stopped,
and only the db 2 operations are rolled back, while db operations 1 are committed to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
and only the db 2 operations are rolled back, while db operations 1 are committed to
and only the db operations 2 are rolled back, while db operations 1 are committed to

docs/sessions_management.rst Outdated
def index():
# db operations 1 ....
try:
db.session.begin_nested():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
db.session.begin_nested():
with db.session.begin_nested():

@mvidalgarcia mvidalgarcia removed their assignment Nov 20, 2019
@ntarocco ntarocco self-requested a review November 20, 2019 14:32
@ntarocco ntarocco moved this from Pending review to Pending merge in Sprint Week 46-47 (2019) - V3.2 Release Nov 20, 2019
@kpsherva kpsherva merged commit 2f59c21 into inveniosoftware:master Nov 20, 2019
Sprint Week 46-47 (2019) - V3.2 Release automation moved this from Pending merge to Done Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvidalgarcia mvidalgarcia mvidalgarcia left review comments

@equadon equadon equadon left review comments

@ntarocco ntarocco ntarocco approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
Sprint Week 46-47 (2019) - V3.2 Release
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

docs: add reference about session management
5 participants
@roksys @ntarocco @mvidalgarcia @equadon @kpsherva