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-6436] Create & Automate docs on Airflow Configs #7015

Merged
merged 2 commits into from
Jan 3, 2020

Conversation

kaxil
Copy link
Member

@kaxil kaxil commented Jan 3, 2020

This PR aims to automate the creation of Config docs and add pre-commit hooks that would also serve as tests.

Preview:

Also, add a structure to the documentation by adding the following sections:

  • Description
  • Example
  • Type
  • Default
  • Version the config was added in (for new configs) (Example: link)
    image

for each config

A user would only need to modify at a single place: config.yml, changes in default_airflow.cfg would be auto-generated using the pre-commit hook.


Link to JIRA issue: https://issues.apache.org/jira/browse/AIRFLOW-6436

  • Description above provides context of the change
  • Commit message starts with [AIRFLOW-NNNN], where 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, no JIRA issue is needed. Commit message starts [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.

@kaxil
Copy link
Member Author

kaxil commented Jan 3, 2020

This PR needs a bit of polishing but its overall logic or structure would remain same.

@codecov-io
Copy link

codecov-io commented Jan 3, 2020

Codecov Report

Merging #7015 into master will decrease coverage by 0.42%.
The diff coverage is 4.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7015      +/-   ##
==========================================
- Coverage   84.85%   84.42%   -0.43%     
==========================================
  Files         679      680       +1     
  Lines       38536    38725     +189     
==========================================
- Hits        32698    32692       -6     
- Misses       5838     6033     +195
Impacted Files Coverage Δ
airflow/utils/config_yaml_to_cfg.py 0% <0%> (ø)
airflow/configuration.py 91.43% <33.33%> (-1.22%) ⬇️
airflow/kubernetes/volume_mount.py 44.44% <0%> (-55.56%) ⬇️
airflow/kubernetes/volume.py 52.94% <0%> (-47.06%) ⬇️
airflow/kubernetes/pod_launcher.py 45.25% <0%> (-46.72%) ⬇️
airflow/executors/sequential_executor.py 56% <0%> (-32%) ⬇️
airflow/kubernetes/refresh_config.py 50.98% <0%> (-23.53%) ⬇️
...rflow/contrib/operators/kubernetes_pod_operator.py 78.75% <0%> (-20%) ⬇️
airflow/utils/helpers.py 74.5% <0%> (-7.19%) ⬇️
airflow/utils/dag_processing.py 80.95% <0%> (-7.05%) ⬇️
... and 60 more

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 0f777f7...ef62156. Read the comment docs.

@potiuk
Copy link
Member

potiuk commented Jan 3, 2020

Love it. I will take a look latere. Akls @dimberman -> i think we might want to merge your PR #6983 with this one.

@dimberman
Copy link
Contributor

This is fantastic. 10/10 @kaxil!

@kaxil kaxil merged commit a94d82f into apache:master Jan 3, 2020
@kaxil kaxil deleted the automate-configs branch January 3, 2020 17:47
@kaxil
Copy link
Member Author

kaxil commented Jan 3, 2020

Going to follow this up with some polishing up PRs.

So, please feel free to add comments on this PR, I will take care of all the comments and fix them in the follow-up PR

@ryw
Copy link
Member

ryw commented Jan 3, 2020

Very nice :)

@dimberman dimberman mentioned this pull request Jan 3, 2020
6 tasks
"""


def default_config_yaml() -> dict:
Copy link
Member

Choose a reason for hiding this comment

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

Should we really do this when loading this module? Maybe you can create a function that will only be called in specific cases? If it is an executable file then it should have the following code at the end of the file

if __name__ == '__main__':
    do_some_magic();

Should it also be in the airflow directory? In my opinion, it looks better in the tests or scripts directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree, updating it in #7036

@denjas
Copy link

denjas commented Feb 13, 2020

Hi!
Latest commit - 9b5dbf9#diff-bbf16e7665ac448883f2ceeb40db35cdR444
in airflow 1.10.8 (1.10.9)
does not allow changing worker concurrency through CLI interface

galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020
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.

7 participants