Re: Transaction APIs do not consult the DB router to choose DB connection

[N Aditya]

Hey Simon, 

It would be possible to write a method as you've suggested above. But it 
entails the following problems:
1. There would have to be a wrapper for each of the transaction APIs, not 
just transaction.atomic since all of them take the using kwarg.
2.  I'm trying to build a library that helps achieve multi-tenancy (the 
isolated DB approach) by dynamically routing DB queries to the appropriate 
databases at runtime. For more info, check this out -> PyCon India 2021 
proposal 
.
 
If that be the case, for any new code base, I could expose a bunch of 
methods as suggested above and ask them to incorporate it. But for 
older/existing code bases which already directly use various transaction 
APIs, it would be hard to replace all lines with another method. 

By inspecting code, I figured that there is this method named 
get_connection in the transaction.py module which is being used by all the 
other transaction APIs for fetching the DB connection. Currently I've 
patched this method with my own to achieve the functionality mentioned 
above. Might I suggest an alternate implementation for this method as 
follows:

#settings.py

TRANSACTION_DB_SELECTOR = "path_to_some_callable"

#transaction.py
...
transaction_db_selector = import_string(settings.TRANSACTION_DB_SELECTOR)
def get_connection():
if transaction_db_selector:
using = transaction_db_selector()
if using is None:
using = DEFAULT_DB_ALIAS
return connections[using]

Let me know your thoughts on this. 

Regards, 
Aditya N

On Friday, May 28, 2021 at 10:05:26 AM UTC+5:30 charettes wrote:

> Ticket that directed to the mailing list for wider feedback 
> https://code.djangoproject.com/ticket/32788
>
> ---
>
> Can you think of places where this db_for_transaction hook would differ 
> in any way from what db_for_write returns? That's what Django uses 
> internally in such instances
>
>1. ​
>
> https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/base.py#L745-L760
>  
>
> 
>  
>2. ​
>
> https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/base.py#L950
>  
>
> 
>  
>3. ​
>
> https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/deletion.py#L400
>  
>
> 
>  
>
> I get that your asking for a way to avoid explicitly passing atomic(using) 
> all over the place but I'm having a hard time figuring out in which cases a 
> db_for_transaction hook, which cannot be provided any contextual details 
> like other router methods do, can take an an advised decision without 
> relying on contextual/global state.
>
> Is there a reason you can't replace your wide spread internal uses of 
> atomic by something along these lines
>
> def routed_atomic(savepoint=True, durable=False):
>
> using = get_global_using()
>
> return transaction.atomic(using, savepoint=savepoint, durable=durable)
>
> Cheers,
>
> Simon
> Le jeudi 27 mai 2021 à 12:18:50 UTC-4, gojeta...@gmail.com a écrit :
>
>> From the Django docs, for any ORM query made, the DB alias to use is 
>> determined by the following rules:
>>
>>- Checks if the using keyword is used either as a parameter in the 
>>function call or chained with QuerySet.
>>- Consults the DB routers in order until a match is found.
>>- Falls back to the default router which always returns default as 
>>the alias.
>>
>> Using the router scheme works perfectly for ORM queries. However, when it 
>> comes to using transaction APIs ​(like the transaction.atomic construct), 
>> it becomes mandatory to specify the *using* kwarg explicitly in all of 
>> its publicly exposed APIs if a DB other than the default alias has to be 
>> chosen.
>>
>> To illustrate why this is a problem, consider the following scenario:
>>
>>- A DB router exists such that it directs queries to a specific 
>>database based on a value set in *thread-local* storage by a 
>>middleware.
>>- When ORM queries from within the view are fired, they automatically 
>>get forwarded to the right DB *without explicitly citing* the using 
>> keyword 
>>owing to the router.
>>- But if the transaction.atomic construct is used, the using keyword 
>>would have to be explicitly specified each time. While this might seem 
>>fine, it creates the following problems:
>>   1. For large code bases, it becomes highly unwieldy to make sure 
>>   that the *using* keyword has been mentioned in every transaction 
>> 

Re: Transaction APIs do not consult the DB router to choose DB connection

['Adam Johnson' via Django developers (Contributions to Django itself)]

Aditya - you didn't answer Simon's first question: "Can you think of places
where this db_for_transaction hook would differ in any way from what
db_for_write returns?" I think this is the crux of the issue. atomic does
already call DB routers - in what case could you need to return a different
result for atomic() than for a write query?

On Fri, 28 May 2021 at 09:17, N Aditya  wrote:

> Hey Simon,
>
> It would be possible to write a method as you've suggested above. But it
> entails the following problems:
> 1. There would have to be a wrapper for each of the transaction APIs, not
> just transaction.atomic since all of them take the using kwarg.
> 2.  I'm trying to build a library that helps achieve multi-tenancy (the
> isolated DB approach) by dynamically routing DB queries to the appropriate
> databases at runtime. For more info, check this out -> PyCon India 2021
> proposal
> .
> If that be the case, for any new code base, I could expose a bunch of
> methods as suggested above and ask them to incorporate it. But for
> older/existing code bases which already directly use various transaction
> APIs, it would be hard to replace all lines with another method.
>
> By inspecting code, I figured that there is this method named
> get_connection in the transaction.py module which is being used by all the
> other transaction APIs for fetching the DB connection. Currently I've
> patched this method with my own to achieve the functionality mentioned
> above. Might I suggest an alternate implementation for this method as
> follows:
>
> #settings.py
>
> TRANSACTION_DB_SELECTOR = "path_to_some_callable"
>
> #transaction.py
> ...
> transaction_db_selector = import_string(settings.TRANSACTION_DB_SELECTOR)
> def get_connection():
> if transaction_db_selector:
> using = transaction_db_selector()
> if using is None:
> using = DEFAULT_DB_ALIAS
> return connections[using]
>
> Let me know your thoughts on this.
>
> Regards,
> Aditya N
>
> On Friday, May 28, 2021 at 10:05:26 AM UTC+5:30 charettes wrote:
>
>> Ticket that directed to the mailing list for wider feedback
>> https://code.djangoproject.com/ticket/32788
>>
>> ---
>>
>> Can you think of places where this db_for_transaction hook would differ
>> in any way from what db_for_write returns? That's what Django uses
>> internally in such instances
>>
>>1. ​
>>
>> https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/base.py#L745-L760
>>
>> 
>>2. ​
>>
>> https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/base.py#L950
>>
>> 
>>3. ​
>>
>> https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/deletion.py#L400
>>
>> 
>>
>> I get that your asking for a way to avoid explicitly passing
>> atomic(using) all over the place but I'm having a hard time figuring out
>> in which cases a db_for_transaction hook, which cannot be provided any
>> contextual details like other router methods do, can take an an advised
>> decision without relying on contextual/global state.
>>
>> Is there a reason you can't replace your wide spread internal uses of
>> atomic by something along these lines
>>
>> def routed_atomic(savepoint=True, durable=False):
>>
>> using = get_global_using()
>>
>> return transaction.atomic(using, savepoint=savepoint, durable=durable)
>>
>> Cheers,
>>
>> Simon
>> Le jeudi 27 mai 2021 à 12:18:50 UTC-4, gojeta...@gmail.com a écrit :
>>
>>> From the Django docs, for any ORM query made, the DB alias to use is
>>> determined by the following rules:
>>>
>>>- Checks if the using keyword is used either as a parameter in the
>>>function call or chained with QuerySet.
>>>- Consults the DB routers in order until a match is found.
>>>- Falls back to the default router which always returns default as
>>>the alias.
>>>
>>> Using the router scheme works perfectly for ORM queries. However, when
>>> it comes to using transaction APIs ​(like the transaction.atomic
>>> construct), it becomes mandatory to specify the *using* kwarg
>>> explicitly in all of its publicly exposed APIs if a DB other than
>>> the default alias has to be chosen.
>>>
>>> To illustrate why this is a problem, consider the following scenario:
>>>
>>>- A DB router exists such that it directs queries to a specific
>>>database based on a value set in *thread-local* storage by a
>>>middleware.
>>>- When ORM queries from within the view are fired, they
>>>auto

Re: Transaction APIs do not consult the DB router to choose DB connection

[N Aditya]

Hey Adam, 

"atomic does already call DB routers" -> Firstly after reading code, I 
don't think the transaction APIs consult the routers. Secondly, I think I 
already answered it in the initial discussion.

FYI from message-1:


   1. Having a separate method for transaction is good because it need not 
   be mangled up with the other methods i.e db_for_read / db_for_write as they 
   clearly indicate certain operational semantics which aren't tied to a 
   transaction since it can have both reads and writes within it. 
   2. Also, if in the future, there is some special info that can be 
   delivered to the transaction based on which a decision regd which DB to use 
   could be made, then it would be cleanly isolated into its own method.

Regards, 
Aditya N

On Friday, May 28, 2021 at 2:03:16 PM UTC+5:30 Adam Johnson wrote:

> Aditya - you didn't answer Simon's first question: "Can you think of 
> places where this db_for_transaction hook would differ in any way from 
> what db_for_write returns?" I think this is the crux of the issue. atomic 
> does already call DB routers - in what case could you need to return a 
> different result for atomic() than for a write query?
>
> On Fri, 28 May 2021 at 09:17, N Aditya  wrote:
>
>> Hey Simon, 
>>
>> It would be possible to write a method as you've suggested above. But it 
>> entails the following problems:
>> 1. There would have to be a wrapper for each of the transaction APIs, not 
>> just transaction.atomic since all of them take the using kwarg.
>> 2.  I'm trying to build a library that helps achieve multi-tenancy (the 
>> isolated DB approach) by dynamically routing DB queries to the appropriate 
>> databases at runtime. For more info, check this out -> PyCon India 2021 
>> proposal 
>> .
>>  
>> If that be the case, for any new code base, I could expose a bunch of 
>> methods as suggested above and ask them to incorporate it. But for 
>> older/existing code bases which already directly use various transaction 
>> APIs, it would be hard to replace all lines with another method. 
>>
>> By inspecting code, I figured that there is this method named 
>> get_connection in the transaction.py module which is being used by all the 
>> other transaction APIs for fetching the DB connection. Currently I've 
>> patched this method with my own to achieve the functionality mentioned 
>> above. Might I suggest an alternate implementation for this method as 
>> follows:
>>
>> #settings.py
>>
>> TRANSACTION_DB_SELECTOR = "path_to_some_callable"
>>
>> #transaction.py
>> ...
>> transaction_db_selector = import_string(settings.TRANSACTION_DB_SELECTOR)
>> def get_connection():
>> if transaction_db_selector:
>> using = transaction_db_selector()
>> if using is None:
>> using = DEFAULT_DB_ALIAS
>> return connections[using]
>>
>> Let me know your thoughts on this. 
>>
>> Regards, 
>> Aditya N
>>
>> On Friday, May 28, 2021 at 10:05:26 AM UTC+5:30 charettes wrote:
>>
>>> Ticket that directed to the mailing list for wider feedback 
>>> https://code.djangoproject.com/ticket/32788
>>>
>>> ---
>>>
>>> Can you think of places where this db_for_transaction hook would differ 
>>> in any way from what db_for_write returns? That's what Django uses 
>>> internally in such instances
>>>
>>>1. ​
>>>
>>> https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/base.py#L745-L760
>>>  
>>>
>>> 
>>>  
>>>2. ​
>>>
>>> https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/base.py#L950
>>>  
>>>
>>> 
>>>  
>>>3. ​
>>>
>>> https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/deletion.py#L400
>>>  
>>>
>>> 
>>>  
>>>
>>> I get that your asking for a way to avoid explicitly passing 
>>> atomic(using) all over the place but I'm having a hard time figuring 
>>> out in which cases a db_for_transaction hook, which cannot be provided 
>>> any contextual details like other router methods do, can take an an advised 
>>> decision without relying on contextual/global state.
>>>
>>> Is there a reason you can't replace your wide spread internal uses of 
>>> atomic by something along these lines
>>>
>>> def routed_atomic(savepoint=True, durable=False):
>>>
>>> using = get_global_using()
>>>
>>> return transaction.atomic(using, savepoint=savepoint, 
>>> durable=durable)
>>>
>>> Cheers,
>>>
>>> Simon
>>> Le jeudi 27 mai 2021 à 12:18:50 UTC-4, gojeta...@gmail.com a écrit :
>>>
 From the Dj

Re: Transaction APIs do not consult the DB router to choose DB connection

[N Aditya]

Hey Adam, 

Also, after giving it a bit of thought, I figured that integrating this 
logic with the routers framework isn't entirely necessary. 
So I came up with another perspective to the solution as well which I've 
illustrated in message-3 of this thread.

Both approaches work for me. 

Let me know your thoughts on the same.

Regards, 
Aditya N

On Friday, May 28, 2021 at 2:55:02 PM UTC+5:30 N Aditya wrote:

> Hey Adam, 
>
> "atomic does already call DB routers" -> Firstly after reading code, I 
> don't think the transaction APIs consult the routers. Secondly, I think I 
> already answered it in the initial discussion.
>
> FYI from message-1:
>
>
>1. Having a separate method for transaction is good because it need 
>not be mangled up with the other methods i.e db_for_read / db_for_write as 
>they clearly indicate certain operational semantics which aren't tied to a 
>transaction since it can have both reads and writes within it. 
>2. Also, if in the future, there is some special info that can be 
>delivered to the transaction based on which a decision regd which DB to 
> use 
>could be made, then it would be cleanly isolated into its own method.
>
> Regards, 
> Aditya N
>
> On Friday, May 28, 2021 at 2:03:16 PM UTC+5:30 Adam Johnson wrote:
>
>> Aditya - you didn't answer Simon's first question: "Can you think of 
>> places where this db_for_transaction hook would differ in any way from 
>> what db_for_write returns?" I think this is the crux of the issue. 
>> atomic does already call DB routers - in what case could you need to return 
>> a different result for atomic() than for a write query?
>>
>> On Fri, 28 May 2021 at 09:17, N Aditya  wrote:
>>
>>> Hey Simon, 
>>>
>>> It would be possible to write a method as you've suggested above. But it 
>>> entails the following problems:
>>> 1. There would have to be a wrapper for each of the transaction APIs, 
>>> not just transaction.atomic since all of them take the using kwarg.
>>> 2.  I'm trying to build a library that helps achieve multi-tenancy (the 
>>> isolated DB approach) by dynamically routing DB queries to the appropriate 
>>> databases at runtime. For more info, check this out -> PyCon India 2021 
>>> proposal 
>>> .
>>>  
>>> If that be the case, for any new code base, I could expose a bunch of 
>>> methods as suggested above and ask them to incorporate it. But for 
>>> older/existing code bases which already directly use various transaction 
>>> APIs, it would be hard to replace all lines with another method. 
>>>
>>> By inspecting code, I figured that there is this method named 
>>> get_connection in the transaction.py module which is being used by all the 
>>> other transaction APIs for fetching the DB connection. Currently I've 
>>> patched this method with my own to achieve the functionality mentioned 
>>> above. Might I suggest an alternate implementation for this method as 
>>> follows:
>>>
>>> #settings.py
>>>
>>> TRANSACTION_DB_SELECTOR = "path_to_some_callable"
>>>
>>> #transaction.py
>>> ...
>>> transaction_db_selector = import_string(settings.TRANSACTION_DB_SELECTOR)
>>> def get_connection():
>>> if transaction_db_selector:
>>> using = transaction_db_selector()
>>> if using is None:
>>> using = DEFAULT_DB_ALIAS
>>> return connections[using]
>>>
>>> Let me know your thoughts on this. 
>>>
>>> Regards, 
>>> Aditya N
>>>
>>> On Friday, May 28, 2021 at 10:05:26 AM UTC+5:30 charettes wrote:
>>>
 Ticket that directed to the mailing list for wider feedback 
 https://code.djangoproject.com/ticket/32788

 ---

 Can you think of places where this db_for_transaction hook would 
 differ in any way from what db_for_write returns? That's what Django 
 uses internally in such instances

1. ​

 https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/base.py#L745-L760
  

 
  
2. ​

 https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/base.py#L950
  

 
  
3. ​

 https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/deletion.py#L400
  

 
  

 I get that your asking for a way to avoid explicitly passing 
 atomic(using) all over the place but I'm having a hard time figuring 
 out in which cases a db_for_transaction hook, which cannot be provided 
 any contextual deta