Skip to content

Conversation

@kaxil
Copy link
Member

@kaxil kaxil commented Apr 10, 2021

closes #11704

When DAG Serialization is enabled for Airflow >= 1.10.10, <2.0, we don't need to run git-sync container for Webserver as DAGs are fetched from Database. DAG Serialization is enabled for Airflow > 2 so users should be able to stop running §git-sync` containers for Webserver when by running:

helm upgrade airflow . \
  --set dags.gitSync.enabled=true \
  --set dags.gitSync.excludeWebserver=true

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

@kaxil kaxil changed the title Chart: Disable git-sync when DAG serialization is enabled Chart: Allow disabling git-sync when DAG Serialization is enabled Apr 10, 2021
@kaxil kaxil force-pushed the add-exclude-webserver-gitsync branch from 32a92f7 to 5730690 Compare April 10, 2021 21:18
Copy link
Member

Choose a reason for hiding this comment

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

For Airflow 1.10.9 and earlier, DAG files is still needed for DAG code ttab. For Airflow 1.10.10 and newer (including 2.0.0), it is needed to enablestore_dag_code to True (default value: False). DAG Serialization is not enough.
http://airflow.apache.org/docs/apache-airflow/2.0.1/configurations-ref.html#store-dag-code

Copy link
Member Author

@kaxil kaxil Apr 11, 2021

Choose a reason for hiding this comment

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

We had removed the default and have a fallback of True for store_dag_code for Airflow >=1.10.11

airflow/airflow/settings.py

Lines 460 to 462 in 5da8319

# Whether to persist DAG files code in DB. If set to True, Webserver reads file contents
# from DB instead of trying to access files in a DAG folder.
STORE_DAG_CODE = conf.getboolean("core", "store_dag_code", fallback=True)

# Whether to persist DAG files code in DB.
# If set to True, Webserver reads file contents from DB instead of
# trying to access files in a DAG folder.
# Example: store_dag_code = False
# store_dag_code =

Same for 1.10.11:

https://github.com/apache/airflow/blob/1.10.11/airflow/config_templates/default_airflow.cfg#L237-L242

where the value is same as store_serialized_dags:

https://github.com/apache/airflow/blob/1.10.11/airflow/settings.py#L431-L434

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated the description in fb10b09

@kaxil kaxil force-pushed the add-exclude-webserver-gitsync branch from 5730690 to fb10b09 Compare April 11, 2021 11:14
@kaxil kaxil requested a review from mik-laj April 11, 2021 11:15
@github-actions
Copy link

The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the workflow link to check the reason.

@kaxil kaxil force-pushed the add-exclude-webserver-gitsync branch from fb10b09 to 0ea925f Compare April 11, 2021 11:49
@github-actions
Copy link

The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the workflow link to check the reason.

@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Apr 11, 2021
Co-authored-by: Kamil Breguła <mik-laj@users.noreply.github.com>
@ashb
Copy link
Member

ashb commented Apr 12, 2021

Could you update this PR title and description please?

@kaxil kaxil changed the title Chart: Allow disabling git-sync when DAG Serialization is enabled Chart: Allow disabling git-sync for Webserver Apr 12, 2021
@kaxil kaxil merged commit 30c6300 into apache:master Apr 12, 2021
@kaxil kaxil deleted the add-exclude-webserver-gitsync branch April 12, 2021 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:helm-chart Airflow Helm Chart full tests needed We need to run full set of tests for this PR to merge kind:documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Disable DAG sync when DAG serialization is enabled in the Helm Chart

3 participants