Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#15243 closed (fixed)

commit_unless_managed clarification for multiple databases in the docs

Reported by: toqueteos@… Owned by: Jason Kotenko
Component: Documentation Version: 1.2
Severity: Keywords: transaction, commit_unless_managed, raw sql, multiple databases, easy-pickings
Cc: toqueteos@…, Jason Kotenko Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

First ticket of mine here guys.

( Please be nice )

Today I was working with a two databases setup: default and my_other_db.

I wrote a function to execute some queries on my_other_db.

Example code:

def email_update(email, password):
    """
    Update a email account from the database with a new password.
    """
    cursor = connections["my_other_db"].cursor()
    query = "UPDATE users SET password=ENCRYPT(%s) WHERE email=%s"
    # Perform the query
    cursor.execute(query, [password, email])
    # Hey Django, ensure changes are done to the DB
    transaction.commit_unless_managed()

So the problem is that the last line does nothing.

Well, it does something, a rollback on the query because I'm not hitting the right db, the one I chose from the connections dict.

Specifying, again, the database alias name on the commit_unless_managed method, with the using keyword argument makes the function to work.

Example:

transaction.commit_unless_managed(using="my_other_db")

I think, a tiny one-two lines note placed below the third paragraph of the section called Executing custom SQL directly at http://docs.djangoproject.com/en/dev/topics/db/sql/#executing-custom-sql-directly should be ok.

Hope it helps.


Related:

Where's the DRY principle of Django here?

That transaction should look for the database alias used with the cursor, right?

Attachments (1)

django_15243.diff (415 bytes) - added by Jason Kotenko 6 years ago.
SVN Patch

Download all attachments as: .zip

Change History (6)

comment:1 Changed 6 years ago by Gabriel Hurley

Keywords: easy-pickings added
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

Thanks for the report!

As the docs you referenced note: "The object django.db.connection represents the default database connection, and django.db.transaction represents the default database transaction." The important point is that they use the default database connection. If you are manually using a different cursor selected from the available connections there's no seamless way for the transaction machinery to know which is the appropriate connection to commit or roll back. Making it "guess" at transactions would cause more bugs for unwitting users than forcing people to be explicit.

I agree the docs there ought to remind people of this fact, and I think the fix is extremely simple. Simply adding two more lines to the example at the end of that section so it reads more like this ought to make it clear:

from django.db import connections
cursor = connections['my_db_alias'].cursor()
# Your code here...
transaction.commit_unless_managed(using="my_db_alias")

That way the example which shows you how to select another DB also shows you how to commit to it.

comment:2 Changed 6 years ago by Jason Kotenko

Owner: changed from nobody to Jason Kotenko
Status: newassigned

Changed 6 years ago by Jason Kotenko

Attachment: django_15243.diff added

SVN Patch

comment:3 Changed 6 years ago by Jason Kotenko

Cc: Jason Kotenko added
Has patch: set

Two lines of documentation mentioned above have been added. Not promoting to "Ready for Checkin" because this is my first ticket and I don't want to make a stupid mistake. Thanks.

comment:4 Changed 6 years ago by Alex Gaynor

Resolution: fixed
Status: assignedclosed

In [15489]:

Fixed #15243 -- More clearly document that the transaction functions needs to be called with a using parameter to work with a non-default database. Thanks to Jason Kotenko for the patch.

comment:5 Changed 6 years ago by Alex Gaynor

In [15490]:

[1.2.X] Fixed #15243 -- More clearly document that the transaction functions needs to be called with a using parameter to work with a non-default database. Thanks to Jason Kotenko for the patch. Backport of [15489].

Note: See TracTickets for help on using tickets.
Back to Top