potiuk commented on PR #53149:
URL: https://github.com/apache/airflow/pull/53149#issuecomment-3065647456

   > The other thing that worries me about "just" having `airflow.sdk.logging` 
via symlinks is it might be too easy for someone to make a change for one side 
or the other that causes problems without thinking about it, as I think in the 
editor it would just show up and be edited like a normal file.
   
   This is where tests should catch things, and I am not sure if people will 
think about this aspect anyway.  It is surprisingly difficult to reason about 
such "different users" when you are "another users" and not "maintainer" of the 
shared code anyway. It requires complex context switching and will only happen 
if have been burnt by it already.
   
   For example when you look at common.sql experience of ours, it's very 
difficult to reason about impact it might have if you are just working on a 
"single user" side - and we had numerous cases where we found things at CI time 
or review time or even after release. And the code for common.sql was separate 
from the beginning, so I am not sure if "where the code is" plays big role in 
this. I think what we should do is to have a very good test harness - both for 
the common code (intra tests) and for the user side (i.e. using the 
functionality by all "users" of the common code). We've chosen common.sql also 
as a bit of a playground on how things will work when we have a common library 
like this and you could see it in the history of fixes of it. Just a random 
selection of issues caused by this:
   
   * https://github.com/apache/airflow/pull/23816
   * https://github.com/apache/airflow/pull/23971 - 91 comments resulting from 
reviews and impact on the different users
   * https://github.com/apache/airflow/pull/25713
   * https://github.com/apache/airflow/pull/25939
   
   But with symlinks, we can also do "airflow.sdk._shared.logging" if that is 
something we would like to emphasise. That will also work.
   
   >  I'm cool with symlinks and relative imports too -- I think as you say we 
need to explore them too.
   
   Cool. I will do it and we wil be able to compare.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to