Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#15243 closed (fixed)

commit_unless_managed clarification for multiple databases in the docs

Reported by: toqueteos@… Owned by: jasonkotenko
Component: Documentation Version: 1.2
Severity: Keywords: transaction, commit_unless_managed, raw sql, multiple databases, easy-pickings
Cc: toqueteos@…, jasonkotenko 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 jasonkotenko 5 years ago.
SVN Patch

Download all attachments as: .zip

Change History (6)

comment:1 Changed 5 years ago by gabrielhurley

  • Keywords easy-pickings added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 5 years ago by jasonkotenko

  • Owner changed from nobody to jasonkotenko
  • Status changed from new to assigned

Changed 5 years ago by jasonkotenko

SVN Patch

comment:3 Changed 5 years ago by jasonkotenko

  • Cc jasonkotenko 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 5 years ago by Alex

  • Resolution set to fixed
  • Status changed from assigned to closed

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 5 years ago by Alex

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