Multi database setup w/ read only endpoints

[Amertz08]

We're attempting to migrate our app to use read only db endpoints. In our implementation we decided that the read only endpoint should be the default endpoint i.e. you must explicitly decide you want to write. Which to us seems to make sense. Also we implemented our own database router as follows. Router docs. Which handles the direction of individual models read/writes.

class Router:
    def db_for_write(model, **hints):
        return "writer"

    def db_for_read(model, **hints):
        return "default"

This appears to be perfectly fine for areas where transactions are not used. However when transactions are used we get an error.

raise TransactionManagementError('select_for_update cannot be used outside of a transaction.')
django.db.transaction.TransactionManagementError: select_for_update cannot be used outside of a transaction

This emanates from Oauth2Validator.save_bearer_token which opens a transaction against"default" and at various points will call select_for_update. This can also happen when AbstractRefreshToken.revoke is called as it also opens a transaction.

Since we can't specify the using argument in the transaction.atomic we have resorted to implementing our own validator and refresh token model. Overriding the methods and copy pasting the original implementation with one that allows us to specify the using argument. For obvious reasons (we don't want to have to validate the implementation hasn't changed each oauth toolkit release) this is not ideal.

That all being said my initial questions are

What ways have other users solved this?

Is there a general Django convention for transaction connection management? e.g. assume you can always write against default

[MCOngTreal]

Is there any solutions for this issue?
I'm getting django.db.transaction.TransactionManagementError: select_for_update cannot be used outside of a transaction
when using DATABASE_ROUTERS Automatic database routing.

if is using "default", there will be no issue, but once using other naming, it will throw error mentioned above

[ippeiukai]

AbstractRefreshToken.revoke as well.

[ippeiukai]

This seems to work for now.
(just specify the patched OAuth2Validator class as OAUTH2_VALIDATOR_CLASS)

from django.db import transaction
from oauth2_provider.models import AbstractRefreshToken
from oauth2_provider.oauth2_validators import OAuth2Validator as orig_OAuth2Validator
from wrapt import patch_function_wrapper

# https://github.com/jazzband/django-oauth-toolkit/issues/868

NON_DEFAULT_DB = 'writer'

class OAuth2Validator(orig_OAuth2Validator):
    @transaction.atomic(using=NON_DEFAULT_DB)
    def save_bearer_token(self, token, request, *args, **kwargs):
        return super().save_bearer_token(token, request, *args, **kwargs)

    @transaction.atomic(using=NON_DEFAULT_DB)
    def _save_id_token(self, jti, request, expires, *args, **kwargs):
        return super()._save_id_token(jti, request, expires, *args, **kwargs)


@patch_function_wrapper(AbstractRefreshToken, 'revoke')
def revoke(wrapped, instance, args, kwargs):
    with transaction.atomic(using=NON_DEFAULT_DB):
        return wrapped(*args, **kwargs)

[n2ygk]

@ippeiukai If this looks like something that should be a fix to the library, please submit a PR!

[ippeiukai]

Thanks! I’ll give it a go when I have a time.

Any idea how it should be done by the way?
I could add a config to the settings, but it feels wrong. There must be a way to automatically find out which database particular model writes to. 🤔

[n2ygk]

@ippeiukai 🤷 replace the @transaction.atomic decorator with the context manager flavor so you can figure out the database from the token's model for using=? Not really sure as I've not used database routers.

See https://docs.djangoproject.com/en/4.0/topics/db/transactions/#managing-database-transactions

[ippeiukai]

Time ran out after a bit of desk research; theoretically this looks like how it should be done:

• with a model

  from django.db import transaction, router
  with transaction.atomic(using=router.db_for_write(model)):

• with an instance

  from django.db import transaction, router
  with transaction.atomic(using=router.db_for_write(instance._meta.model, instance=instance)):

[dopry]

This feels like something that should be specified upstream in Django. I don't think this something we an easily generalize in DOT.

[shaleh]

Django does not support setting something other than 'default' as 'default'. From their docs:

If the concept of a default database doesn’t make sense in the context of your project, you need to be careful to always specify the database that you want to use.

This project not allowing users to specify where the Tokens live and then not honoring the routers when we do is fundamentally the problem. Wrapping the code in a custom Validator does work but it spawns two Transactions which is not ideal. Moving the transactions down into context managers is a solid plan. Refactoring the Validator slightly so the work of save_bearer_token is in a helper function would allow this easily.