-
Notifications
You must be signed in to change notification settings - Fork 16.6k
Improve dag_bundle_config_list Configuration in Airflow Helm Chart#60645
Improve dag_bundle_config_list Configuration in Airflow Helm Chart#60645jscheffl merged 5 commits intoapache:mainfrom
Conversation
Overview
Related Issue: #60630
Improve dag_bundle_config_list Configuration in Airflow Helm Chart
This PR improves the dag_bundle_config_list configuration in the Airflow Helm Chart by allowing users to define DAG bundle configurations in a structured YAML format instead of requiring a JSON string directly in the values file.
Changes
- Added a new
dag_bundlessection underdagProcessorinvalues.yamlthat allows users to define DAG bundles in a structured YAML format - Created a Helm template helper function
dag_bundlesthat automatically converts the YAML structure to the required JSON string format - Set default value for
dagProcessor.dag_bundleswith a local DAG bundle configuration - Updated
config.dag_processor.dag_bundle_config_listto use the helper function - Moved
dag_bundlesconfiguration underdagProcessorsection for better logical grouping
Benefits
- Better Readability: Long JSON strings in YAML files are difficult to read and maintain. The new structured format is much more readable.
- Less Error-Prone: Manual JSON string construction is prone to syntax errors. The YAML format reduces the chance of errors.
- Better IDE Support: IDEs can provide proper syntax highlighting, validation, and autocomplete for YAML structures.
- Easier Maintenance: Adding, removing, or modifying bundle configurations is now straightforward.
- Better Organization: DAG bundle configuration is now logically grouped under the
dagProcessorsection.
Usage
Users can now define DAG bundles like this:
dagProcessor:
dag_bundles:
- name: dags-folder
classpath: "airflow.dag_processing.bundles.local.LocalDagBundle"
kwargs: {}
- name: bundle1
classpath: "airflow.providers.git.bundles.git.GitDagBundle"
kwargs:
git_conn_id: "GITHUB__repo1"
subdir: "dags"
tracking_ref: "main"
refresh_interval: 60
- name: bundle2
classpath: "airflow.providers.git.bundles.git.GitDagBundle"
kwargs:
git_conn_id: "GITHUB__repo2"
subdir: "dags"
tracking_ref: "develop"
refresh_interval: 120
Was generative AI tooling used to co-author this PR?
- Yes (please specify the tool below)
- Cursor
- Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
- For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
- When adding dependency, check compliance with the ASF 3rd Party License Policy.
- For significant user-facing changes create newsfragment:
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.
3c59689 to
61b69a2
Compare
61b69a2 to
0641f54
Compare
jscheffl
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 tests are missing - can you add some please?
Thank you for the feedback. I've updated the PR with the following changes:
|
ecf3957 to
eb4dacb
Compare
choo121600
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.
One small nit suggestion:
we've agreed to refer to DAG as Dag going forward.
It would be great if you could update the occurrences accordingly :)
[VOTE] results as of https://lists.apache.org/thread/0651j4vdgzmlhgndmotvznlg97kyh328
Great catch, thank you for sharing the discussion link. I've gone through the PR and adjusted |
jscheffl
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.
Looks good for me.
* Improve dag_bundle_config_list Configuration in Airflow Helm Chart
* Apply review of @jscheffi - Mark config.dag_processor.dag_bundle_config_list as deprecated
* Apply review of @jscheffi - Add test for dagProcessor.dag_bundles
* Rename dag_bundles to dagBundleConfigList for consistency
* Apply review of @choo121600 - Update term DAG -> Dag
* Improve dag_bundle_config_list Configuration in Airflow Helm Chart
* Apply review of @jscheffi - Mark config.dag_processor.dag_bundle_config_list as deprecated
* Apply review of @jscheffi - Add test for dagProcessor.dag_bundles
* Rename dag_bundles to dagBundleConfigList for consistency
* Apply review of @choo121600 - Update term DAG -> Dag
* Improve dag_bundle_config_list Configuration in Airflow Helm Chart
* Apply review of @jscheffi - Mark config.dag_processor.dag_bundle_config_list as deprecated
* Apply review of @jscheffi - Add test for dagProcessor.dag_bundles
* Rename dag_bundles to dagBundleConfigList for consistency
* Apply review of @choo121600 - Update term DAG -> Dag
* Improve dag_bundle_config_list Configuration in Airflow Helm Chart
* Apply review of @jscheffi - Mark config.dag_processor.dag_bundle_config_list as deprecated
* Apply review of @jscheffi - Add test for dagProcessor.dag_bundles
* Rename dag_bundles to dagBundleConfigList for consistency
* Apply review of @choo121600 - Update term DAG -> Dag
* Improve dag_bundle_config_list Configuration in Airflow Helm Chart
* Apply review of @jscheffi - Mark config.dag_processor.dag_bundle_config_list as deprecated
* Apply review of @jscheffi - Add test for dagProcessor.dag_bundles
* Rename dag_bundles to dagBundleConfigList for consistency
* Apply review of @choo121600 - Update term DAG -> Dag