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

[AIRFLOW-6516] BugFix: airflow.cfg does not exist in Volume Mounts #7109

Merged
merged 2 commits into from
Jan 9, 2020

Conversation

kaxil
Copy link
Member

@kaxil kaxil commented Jan 9, 2020

4748a51 introduced a bug because we were re-assigning the "airflow-config" key in volume_mounts/

Because of this airflow.cfg was not mounted and only airflow_local_settings.py was mounted if the airflow_local_settings_configmap is also set.


Issue link: AIRFLOW-6516

  • Description above provides context of the change
  • Commit message/PR title starts with [AIRFLOW-NNNN]. AIRFLOW-NNNN = JIRA ID*
  • Unit tests coverage for changes (not needed for documentation changes)
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

* For document-only changes commit message can start with [AIRFLOW-XXXX].


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.
Read the Pull Request Guidelines for more information.

apache@4748a51 introduced a bug because we were re-assigning the "airflow-config" key in volume_mounts/

Because of this `airflow.cfg` was not mounted and only `airflow_local_settings.py` was mounted if the `airflow_local_settings_configmap` is also set.
@kaxil kaxil requested a review from dimberman January 9, 2020 01:34
@boring-cyborg boring-cyborg bot added the k8s label Jan 9, 2020
@dimberman
Copy link
Contributor

ohhh ok so you were overwriting airflow-config before. Got it. LGTM

@codecov-io
Copy link

codecov-io commented Jan 9, 2020

Codecov Report

Merging #7109 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #7109   +/-   ##
======================================
  Coverage    85.2%   85.2%           
======================================
  Files         682     682           
  Lines       38996   38996           
======================================
  Hits        33228   33228           
  Misses       5768    5768
Impacted Files Coverage Δ
airflow/kubernetes/worker_configuration.py 99.29% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6676c4...036685d. Read the comment docs.

@kaxil kaxil merged commit a0a02df into apache:master Jan 9, 2020
@kaxil kaxil deleted the fix-configmap-bug branch January 9, 2020 03:15
kaxil added a commit that referenced this pull request Jan 11, 2020
potiuk pushed a commit that referenced this pull request Jan 21, 2020
kaxil added a commit that referenced this pull request Jan 23, 2020
kaxil added a commit to kaxil/airflow that referenced this pull request Feb 24, 2020
dimberman pushed a commit that referenced this pull request Feb 24, 2020
#7518)

* [AIRFLOW-6516] Allow different configmaps for airflow.cfg & airflow_local_settings.py (#7109)

* Update test_worker_configuration.py
galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020
galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020
apache#7518)

* [AIRFLOW-6516] Allow different configmaps for airflow.cfg & airflow_local_settings.py (apache#7109)

* Update test_worker_configuration.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants