Graphene-django: Handling database transactions

[brucedesa]

I noticed that graphene-django doesn't handle database transactions. I believe this is a issue that people should know or be careful about.

The same logic of DRF could be implemented as in the lines below that run in the dispatch method.

https://github.com/encode/django-rest-framework/blob/d985c7cbb999b2bc18a109249c583e91f4c27aec/rest_framework/views.py#L65-L102

Basically on one exception runs:

```python
atomic_requests = settings.get('ATOMIC_REQUESTS', False)
if atomic_requests and connection.in_atomic_block:
transaction.set_rollback(True)
````

In implementing this in graphene-django as we do not have the exception could run the rollback in the error checking of the execution result as in line 171.

https://github.com/graphql-python/graphene-django/blob/968002f1554e3a7a1c0617682be64b67823b2581/graphene_django/views.py#L167-L190

What do you think about this implementation?

[jkimbo]

Wouldn’t it mean that if part of the query/mutation raised an exception then every database change would be rolled back? Also wouldn’t the whole execution need to be wrapped in a transaction atomic block as well?

[brucedesa]

As good practice all requests are within an atomic block this is Django's default behavior.

In case of performance Django has utilities to run mutations without atomicity.

But since i use ATOMIC_REQUESTS=True, and the exceptions are not raised up, graphene-django does not care to make the request atomic and rollback on failure.

[jkimbo]

Ok fair enough, it seems like a good addition to the library. Your suggestion on where to put the transaction rollback is correct. Could you create a PR that adds the rollback under a settings option?

[ace-han]

Hello, any progress on this issue or PR?

[stale[bot]]

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[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

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

[iorlandini]

@jkimbo, @ulgens, @ace-han and @brucedesa I was faced with an issue similar to the one described in this issue and I created the PR #1039 to solve both use cases.