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

savepoint_rollback() after an IntegrityError gives TransactionManagementError #399

Closed
ketanbhatt opened this issue Feb 10, 2016 · 7 comments
Labels
stale Stale bot

Comments

@ketanbhatt
Copy link

Hey,

I noticed that if my model throws an IntegrityError while saving, and we do a savepoint_rollback() after that, it throws TransactionManagementError.
I investigated a little and found that in an atomic() block, Integrity Error causes automatic rollbacks, so calling savepoint_rollback() raises error.

@sergei-maertens
Copy link

+1, running into the same issue at the moment

@bmihelac
Copy link
Member

reference: #377

bmihelac added a commit that referenced this issue Mar 8, 2016
…nagementError #399

wrap save_m2m in transaction.atomic()

references #377
@bmihelac
Copy link
Member

bmihelac commented Mar 8, 2016

This should be fixed in branch 412-drop-django-15-support

https://github.com/django-import-export/django-import-export/tree/412-drop-django-15-support

Can you please test if this works for your case?

@int-ua
Copy link
Contributor

int-ua commented Nov 28, 2017

I'm getting this on 0.6.0. The IntegrityError is raised somewhere on init_instance (still trying to find the exact point).

My case is having a "legal person" model that have O2O field to a Person model and a code field that is preferred for determining the right legal person. I'm trying to determine a Person by multiple fields with my custom widget and today there was two legal person rows with different codes. So it failed since I'm returning a second legal person connected to the same Person. But it doesn't raise the IntegrityError exception in my widget code for some reason. Will continue investigating it tomorrow.

@int-ua
Copy link
Contributor

int-ua commented Nov 29, 2017

AFAIU the reason is that Resource.import_row doesn't account for IntegrityError possibility and this example from Django docs cannot be used:

try:
    with transaction.atomic():
        generate_relationships()
except IntegrityError:
    handle_exception()

So we need to prevent any savepoint_rollback call if there was an IntegrityError since it has been already rolled back. Is that right?

int-ua added a commit to int-ua/django-import-export that referenced this issue Nov 29, 2017
bmihelac added a commit to bmihelac/django-import-export that referenced this issue Nov 30, 2017
Add `atomic_if_using_transaction` context manager and use it instead of
if/else statements.

Wrap `before_import`, `import_row`, `after_import` calls into
`atomic_if_using_transaction`.

Add tests.

Refs: django-import-export#609, django-import-export#399, django-import-export#610
bmihelac added a commit to bmihelac/django-import-export that referenced this issue Dec 4, 2017
Add `atomic_if_using_transaction` context manager and use it instead of
if/else statements.

Wrap `before_import`, `import_row`, `after_import` calls into
`atomic_if_using_transaction`.

Add tests.

Refs: django-import-export#609, django-import-export#399, django-import-export#610
@rhunwicks
Copy link
Contributor

rhunwicks commented Feb 27, 2018

@bmihelac In dd1c085 you removed the exception handling from the call to before_import.

Did you mean to remove the raise() as well as the rollback?

Because what happens now is that if you call import_data with raise_errors=True and before_import has an Exception, then it logs the exception but doesn't raise it. So the Resource then carries on and tries to import all the rows in the dataset, which isn't what we want. The after_import call does raise errors if requested.

I will prepare an MR to add

            if raise_errors:
                raise

to the before_import call.

@stale
Copy link

stale bot commented Feb 11, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Stale bot label Feb 11, 2019
@stale stale bot closed this as completed Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale bot
Projects
None yet
Development

No branches or pull requests

5 participants