-
Notifications
You must be signed in to change notification settings - Fork 6
DOCSP-51036: Transactions #40
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.
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
DOCSP-51036: Transactions #40
Conversation
✅ Deploy Preview for docs-django ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🔄 Deploy Preview for docs-django processing
|
**transactions**. Transactions allow you to run a series of operations | ||
that change data only if the entire transaction is committed. | ||
If any operation in the transaction does not succeed, {+django-odm+} stops the | ||
transaction and discards all data changes before they ever become |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transaction and discards all data changes before they ever become | |
transaction and discards all changes to the data before they ever become |
Consider this for clarity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm with a couple suggestions! great work!
changes are committed MongoDB. | ||
|
||
The following example calls the ``create()`` method within a transaction, | ||
inserting a document into the ``sample_mflix.movies`` collection if the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inserting a document into the ``sample_mflix.movies`` collection if the | |
which inserts a document into the ``sample_mflix.movies`` collection if the |
Run Callbacks After a Transaction | ||
--------------------------------- | ||
|
||
If you want to perform certain actions only if a transaction successfully |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to perform certain actions only if a transaction successfully | |
To perform certain actions only if a transaction successfully |
--------------------------------- | ||
|
||
If you want to perform certain actions only if a transaction successfully | ||
completes, you can use the ``transaction.on_commit()`` method. This method allows you to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
completes, you can use the ``transaction.on_commit()`` method. This method allows you to | |
completes, use the ``transaction.on_commit()`` method. This method allows you to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on_commit()
is a function, not a method.
If you want to perform certain actions only if a transaction successfully | ||
completes, you can use the ``transaction.on_commit()`` method. This method allows you to | ||
register callbacks that run after a transaction is committed to the | ||
database. Pass a function, or any callable object, as an argument to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
database. Pass a function, or any callable object, as an argument to | |
database. To use this method, pass a function, or any callable object, as an argument to |
I think this is what this says
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll keep as is to avoid using too many commas, since I think the "to use..." is implied
) | ||
try: | ||
with transaction.atomic(): | ||
movie.update(title="Jurassic Park I") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
models don't have an update()
method
# start-handle-errors | ||
movie = Movie.objects.get( | ||
title="Jurassic Park", | ||
released=timezone.make_aware(datetime(1993, 6, 11)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably enough to retrieve a move by name (omit released=...
).
runtime=140, | ||
genres=["Horror", "Comedy"] | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chop the blank line?
source/limitations-upcoming.txt
Outdated
@@ -248,8 +250,17 @@ Database and Collection Support | |||
- ✓ | |||
|
|||
* - Transactions | |||
- *Unsupported*. | |||
- ✓ | |||
- *Partially Supported*. You can use {+framework+}'s transactions API with the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not an issue for this PR to tackle, but I think it's a strange choice of words to call it "Partial support" when there are any limitations. To me, "partial support" makes me think "this feature is partially completed and more work will be done later".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm I see what you mean, is "Supported with Limitations" more accurate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I think it would be enough to say, "✓ You use... with the following limitations:" rather than place a double emphasis on limitations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I like ✓ with limitations noted. Partial support has more of a "we may get further on this soon" which I don't think is the case with transactions support.
source/limitations-upcoming.txt
Outdated
- Migration operations do not run inside a transaction, which differs from {+framework+}'s | ||
default migration behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which differs from {+framework+}'s default migration behavior.
This isn't quite accurate. It depends on the database. SQLite & PostgreSQL run operations inside a transaction, but MySQL and Oracle don't. (The relevant feature flag is DatabaseFeatures.can_rollback_ddl
.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this part
|
||
To handle exceptions that occur during a transaction, add error handling | ||
logic around your atomic code block. If you include error handing logic inside | ||
the atomic block, {+framework+} might ignore these errors, resulting in unexpected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the meaning of "{+framework+} might ignore these errors," is probably unclear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some more detail here
the atomic block, {+framework+} might ignore these errors, resulting in unexpected | ||
behavior. | ||
|
||
If a transaction does not succeed, your application does not revert any changes made |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"your application" -> Django
--------------------------------- | ||
|
||
If you want to perform certain actions only if a transaction successfully | ||
completes, you can use the ``transaction.on_commit()`` method. This method allows you to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on_commit()
is a function, not a method.
with transaction.atomic(): | ||
movie.update(title="Jurassic Park I") | ||
except DatabaseError: | ||
movie.update(title="Jurassic Park") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might copy from Django's example and use the value of movie.title
in a line that follows (like if obj.active:
in Django's example) to make it clear why it could be important to revert the value.
{+django-odm+}'s support for the {+framework+} transaction API | ||
has several limitations. To view a list of limitations, see | ||
:ref:`Database and Collection Support <django-feature-compat-db-coll>` | ||
in the Django and MongoDB Feature Compatibility guide. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the Django and MongoDB Feature Compatibility guide. | |
in the {+framework+} and MongoDB Feature Compatibility guide. |
{+framework+} uses errors to determine whether to commit or roll | ||
back a transaction, this can cause unexpected behavior. | ||
|
||
If a transaction does not succeed, Django does not revert any changes made |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a transaction does not succeed, Django does not revert any changes made | |
If a transaction does not succeed, {+framework+} does not revert any changes made |
source/limitations-upcoming.txt
Outdated
@@ -248,8 +250,17 @@ Database and Collection Support | |||
- ✓ | |||
|
|||
* - Transactions | |||
- *Unsupported*. | |||
- ✓ | |||
- *Partially Supported*. You can use {+framework+}'s transactions API with the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I think it would be enough to say, "✓ You use... with the following limitations:" rather than place a double emphasis on limitations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-51036
Staging Links
Self-Review Checklist