Skip to content

POC of a symlink-based code sharing approach. #53417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ashb
Copy link
Member

@ashb ashb commented Jul 16, 2025

Don't read too much in to the specific paths chosen.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:CLI area:DAG-processing area:db-migrations PRs with DB migration area:Scheduler including HA (high availability) scheduler area:Triggerer labels Jul 16, 2025
@ashb
Copy link
Member Author

ashb commented Jul 16, 2025

A possible alternative to #53149

Don't read too much in to the specific paths chosen.
@ashb ashb force-pushed the shared-lib-via-symlinks branch from d177c2d to ee3aecf Compare July 16, 2025 12:49
@uranusjr
Copy link
Member

I wonder how imports between shared modules should work in this approach.

@ashb
Copy link
Member Author

ashb commented Jul 17, 2025

I wonder how imports between shared modules should work in this approach.

As per Jarek here, cross-imports from shared deps would be banned:

You could just open the logging project from Airlow repo and work on it as if it was a completely standalone project - add and run tests etc. and make sure that it has no dependencies to other part of the system (maintaining low cyclomatic complexity).

@potiuk
Copy link
Member

potiuk commented Jul 17, 2025

Yep. I wil take a look tomorrow/over weekend.

I wonder how imports between shared modules should work in this approach.

This could be done as symlinks between shared modules - but I also think such imports should be discouraged. I think each shared module should be independent from each other - because that is what adspds complexithy. For example - rather than reading a configuration from logging module - you should pass a parameter from outside that provides the config.

Generally spekaing - "needin a one shared module to use another shared module" is an antipattern that shold be possible when really needed, but it should be avoided.

This is waht cyclomatic complexity "theory" is about when turned into practice (as far as I understand it). It you look at breeze code this is how the whole package is organized like. there are about 30 independent modules that do not use each other, whenever there is a need to share things, the shared things are moved into a "one level deep" module that is imported by other modules (global constants is a good example of it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API area:CLI area:DAG-processing area:db-migrations PRs with DB migration area:Scheduler including HA (high availability) scheduler area:Triggerer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants