-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Move secrets_masker over to airflow_shared distribution #54449
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
Move secrets_masker over to airflow_shared distribution #54449
Conversation
|
The CI job is finally green: https://github.com/apache/airflow/actions/runs/17154718454/job/48668823304?pr=54449 |
potiuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonderful!
kaxil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments to be addressed but high-level is fine
|
Good lord, that was a big one with changes in many areas including precommits etc! Thanks for your reviews @potiuk and @kaxil! The tooling is awesome (even more so with the improvements now) for shared library bootstrapping! Merging this and will be handling the open comments in #54859 and resolve it soon |
Indeed. I am very happy with the tooling :). And with #54863 we have small common test framework for the shared distributions as well :) |
closes: #54397
Why
As part of #51545, we are trying to avoid code duplication between
airflow-coreandtask-sdk. The secrets masking functionality is widely used so that it can be moved to a shared library. This follows the pattern set by the timezone utilities migration(#53149) and ensures a single source of truth for secrets masking logic across Airflow codebase.High level changes made
1. Created shared library structure
shared/secrets_masker/with standard shared library layoutshared/secrets_masker/pyproject.tomldefiningapache-airflow-shared-secrets_maskertask-sdk/src/airflow/sdk/execution_time/secrets_masker.pytoshared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py2. Updated consuming packages
shared/secrets_maskerto rootpyproject.tomlworkspace memberstask-sdk/src/airflow/sdk/secrets_masker.pythat re-exports shared functionality4. How the updates were made:
check-shared-distributions-structureandcheck-shared-distributions-usagehooksairflow/_shared/secrets_maskerandairflow/sdk/_shared/secrets_maskerpyproject.tomlfiles for wheel packagingSanity tests
Symlink verification
Shared library test in isolation
Import validation
Some tests for peace of mind
DAG used for testing some cases:
Env set:
airflow tasks test subprocess_dag py_fuThis is specifically to test the recent issue: 1f4c55c
Trigger:
DAG:
Logs:
TODO next
^ 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.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.