Code

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#18080 closed Uncategorized (invalid)

@transaction.commit_manually plus context processors doesn't work well

Reported by: jstpierre@… Owned by: nobody
Component: Uncategorized Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Imagine a view that looks like this:

    @transaction.commit_manually
    def create_new_foo(request):
        name = request.GET['name']
        obj = MyFoo(name=name)
        # We need to add the object to a ManyToMany
        # so we need a PK.
        obj.save()
        add_to_many_to_many(obj)
        if obj.is_valid():
            valid = True
            transaction.commit()
        else:
            valid = False
            transaction.rollback()

        return render_to_response("objs/obj_created.html", {'valid': valid,
                                                            'obj': obj})

Now imagine a context processor in another application entirely that looks like this:

    def num_my_bars(request):
        return MyBar.objects.count()

Since num_my_bars hits the database during the render, the transaction will be dirtied and leaving the commit_manually view will fail. A dirty solution may be to do:

        response = render_to_response(...)
        transaction.set_clean()
        return response

in the view, but this is dirty and awkward.

Attachments (0)

Change History (3)

comment:1 Changed 2 years ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

To me it seems the commit_manually decorator works exactly as advertised. The solution is to maybe define another function to do the actual prosessing:

def create_new_foo(request):
    @transaction.commit_manually
    def inner():
        # do stuff
    inner()
    return render_to_response...

I am closing this as invalid. If you want to reopen this please provide more details of what should be fixed. Is documentation erroneous or lacking for example?

comment:2 Changed 2 years ago by Jasper St. Pierre <jstpierre@…>

It could be considered a documentation issue, but the problem is really apparent when mixing and matching external applications - one application that uses commit_manually may not use an internal function.

A solution may be to "disable" transactions when in a call to render, and restore it after the call (instead of an is_automatic boolean, make it a stack, make transaction.commit_manually push onto the stack, push onto the stack while rendering templates and evaluating context processors).

comment:3 Changed 2 years ago by akaariai

My take is that the shown usage of commit_manually is a bug in the user code - it does not take into account that the render_to_response might do queries. The render_to_response is clearly part of the function, and thus the commit_manually applies to it, too.

Disabling transactions as a side-effect of rendering under commit_manually decorator is a big no-no in my opinion.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.