potiuk commented on code in PR #53149: URL: https://github.com/apache/airflow/pull/53149#discussion_r2202729152
########## task-sdk/src/airflow/sdk/_vendor/.gitignore: ########## @@ -0,0 +1 @@ +*/*.py Review Comment: > I guess the main issue with relative imports is are we okay with no imports between shared libs? For example, it might be nice to be able to use the timezone/date parsing stuff in the logging one, but I don't think we can do that when symlinking? I think (that was also my cyclomatic complexity arguments) we should avoid it as much as possible - because then it can very easily lead to circular dependencies (config -> logging - > config that we currently have is a classic example of it). Most of this can be solved by injecting stuff at the higher level - for example during explicit initialization. It makes the "acyclic tree" hierarchy of dependencies between such shared code, which is precisely what low cyclomatic complexity is about. So in a way - adding "friction" for the shared code to use another shared code is likely a good thing. You should **not** be able to use the shared code from another module to easily. But also - it's not impossible in case of symlinks - you can have symlinks from one shared library to the other. That will work exactly the same way as symlinking shared library in the main code. And I think it's the right "amount of friction" - because you not only have to import stuff from the other library, but have to make symlink and it will - much more obviously - raise eyebrows during review - and cases like "logging uses config and config uses logging" is somethign that will be definitely "no go" as it will "visibly" create circular dependencies. -- 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]
