Skip to content
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

Use single source of truth for sensitive config items #31820

Merged

Conversation

dstandish
Copy link
Contributor

Previously we had them defined both in constant and in config.yml.

Now just config.yml

This change does however cause config.yml to be loaded when the sensitive_config_values property is accessed.

Previously we had them defined both in constant and in config.yml.

Now just config.yml

This change does however cause config.yml to be loaded when the sensitive_config_values property is accessed.
@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Jun 9, 2023
Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lazy loading that means it should only matter when someone hits the config page right? Seems good to me - that is as long as we ship the config yaml in the dist, but I'm not sure if we do or don't

tests/core/test_configuration.py Show resolved Hide resolved
@dstandish
Copy link
Contributor Author

Lazy loading that means it should only matter when someone hits the config page right? Seems good to me - that is as long as we ship the config yaml in the dist, but I'm not sure if we do or don't

ah yes, at first glance i thought this property might have gotten loaded up every time airflow conf got loaded but, i think you're right it's only invoked via webserver / configuration view

so i think performance of loading the yaml is non-issue

@dstandish dstandish merged commit cab342e into apache:main Jun 9, 2023
1 check passed
@dstandish dstandish deleted the single-source-truth-sensitive-config branch June 9, 2023 21:38
@jedcunningham jedcunningham added this to the Airflow 2.6.2 milestone Jun 9, 2023
potiuk pushed a commit that referenced this pull request Jun 9, 2023
Previously we had them defined both in constant and in config.yml.

Now just config.yml

(cherry picked from commit cab342e)
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:webserver Webserver related Issues type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants