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.

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

with reversion.create_revision() and my_model.objects.select_related('XXX').update_or_create #570

Closed
murdav opened this issue Aug 22, 2016 · 11 comments

Comments

@murdav
Copy link

murdav commented Aug 22, 2016

Hello,

This is the code:

@transaction.atomic
def my_func():
    """
    """

    #
    # calculate obj
    # calculate obj_dict for update_or_create defaults 
    #

    with reversion.create_revision():
        my_instance, created = my_model.objects.select_related('type')\
                                         .update_or_create(title=obj.title,
                                                           defaults=obj_dict)

        reversion.set_user(user)
        reversion.set_comment("Created revision {0}".format(datetime.now()))

The revision is not saved, but within update_or_create we have obj.save.
Anyway I don't have any error.
Is there any issue having the decorator @transaction.atomic?

Using the Django admin everything works.
Thanks,

D

@etianen
Copy link
Owner

etianen commented Aug 22, 2016

Using transaction.atomic() isn't usually necessary, since
reversion.create_revision() will also wrap the contained code in a
transaction. However, it won't do any harm having it twice.

Is my_model registered with django-reversion?

On Mon, 22 Aug 2016 at 16:25 murdav [email protected] wrote:

Hello,

This is the code:

@transaction.atomic
def my_func():
"""
"""

#
# calculate obj
# calculate obj_dict for update_or_create defaults
#

with reversion.create_revision():
    my_instance, created = my_model.objects.select_related('type')\
                                     .update_or_create(title=obj.title,
                                                       defaults=obj_dict)

    reversion.set_user(user)
    reversion.set_comment("Created revision {0}".format(datetime.now()))

The revision is not saved, but within update_or_create we have obj.save.
Anyway I don't have any error.
Is there any issue having the decorator @transaction.atomic?

Using the Django admin everything works.
Thanks,

D


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#570, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AAJFCKeyM6BRKd8wQm5HKm2pJrnTZNlEks5qib9RgaJpZM4Jp_X3
.

@murdav
Copy link
Author

murdav commented Aug 22, 2016

Dear Author,
Yes, the model is registered using the Version decorator, and using the Django admin it works.
I need to have the transaction decorator for other issues.

@etianen
Copy link
Owner

etianen commented Aug 23, 2016

Out of interest, does replacing the update_or_create code with my_model.create(title=obj.title, **obj_dict) work?

I'm wondering if update_or_create always calls save(), when the model is updated.

@murdav
Copy link
Author

murdav commented Aug 23, 2016

Dear @etianen,

I tried with:

@transaction.atomic
def my_func():
    """
    """

    #
    # calculate obj
    # calculate obj_dict for update_or_create defaults 
    #

    with reversion.create_revision():
        # just to test I did this
        f = MyModel.objects.create(url='http://test.com', title='test rev', inserted_by=user)

        reversion.set_user(user)
        reversion.set_comment("Created revision {0}".format(datetime.now()))

    my_instance = MyModel.objects.latest('created')

But without having success, I got the same app behavior, basically no revision was added, while my_instance is correctly retrieved and used in other parts of the function.

But if I remove the decorator @transaction.atomic it works.
Now what could I do? I need that decorator, because that function does several cascade insert.

D

@etianen
Copy link
Owner

etianen commented Aug 24, 2016

Hrm....

I don't really know what to say. The exact same code is used in the admin
interface.

Try this inside that function:

print(reversion.is_registered(MyModel))

On Tue, 23 Aug 2016 at 14:13 murdav [email protected] wrote:

Dear @etianen https://github.com/etianen,

I tried with:

@transaction.atomic
def my_func():
"""
"""

#
# calculate obj
# calculate obj_dict for update_or_create defaults
#

with reversion.create_revision():
    # just to test I did this
    f = MyModel.objects.create(url='http://test.com', title='test rev', inserted_by=user)

    reversion.set_user(user)
    reversion.set_comment("Created revision {0}".format(datetime.now()))

my_instance = MyModel.objects.latest('created')

But without having success, I got the same app behavior, basically no
revision was added, while my_instance is correctly retrieved and used in
other parts of the function.

Do you have further ideas?
I will try other tests later.

D


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#570 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJFCFbaP7a4kDvZf_ej1JihFlOuAClPks5qivIDgaJpZM4Jp_X3
.

@murdav
Copy link
Author

murdav commented Aug 24, 2016

It returns True. I suppose it is somehow related to have @transaction.atomic also if the Django doc says: "atomic blocks can be nested." there is something that creates troubles.
Or maybe it's a combination of issues, I'm having problems with Taggit (#76) also if I didn't change/dd/delete the keywords and I also have this bug using JSONField (https://code.djangoproject.com/ticket/25532) each time I save a model using the JSONField it adds slashes.
....

@etianen
Copy link
Owner

etianen commented Aug 24, 2016

I think something has gone a bit wrong somewhere, probably in your code,
but maybe not.

Time to debug! Try adding loads of print() statements in the reversion
codebase to pin down whether django-reversion is actually being triggered
by the model's post_save signal.

On Wed, 24 Aug 2016 at 14:52 murdav [email protected] wrote:

It returns True. I suppose it is somehow related to have
@transaction.atomic also if the Django doc says: "atomic blocks can be
nested." there is something that creates troubles.
Or maybe it's a combination of issues, I'm having problems with Taggit (
#76 #76) also if I
didn't change/dd/delete the keywords and I also have this bug using
JSONField (https://code.djangoproject.com/ticket/25532) each time I save
a model using the JSONField it adds slashes.
....


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#570 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJFCIDqxaeQaS0JcIBmFg8dSVOULPFuks5qjEyvgaJpZM4Jp_X3
.

@etianen etianen closed this as completed Nov 28, 2016
@stevekm
Copy link

stevekm commented Sep 2, 2020

Was there a solution to this? I am seeing the same behavior, where a call to

    with reversion.create_revision():
        instance, created = MyModel.objects.update_or_create(sample = sample, defaults = values)

results in the model instance being updated successfully in the database, but reversion does not record a change or new version.

@etianen
Copy link
Owner

etianen commented Sep 2, 2020

Does it work if you use MyModel.objects.create() instead?

@stevekm
Copy link

stevekm commented Sep 5, 2020

It works if I use .create() but that forces me to re-write the logic bundled into update_or_create() which I wanted to use

@etianen
Copy link
Owner

etianen commented Sep 7, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants