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.

Sign up for GitHub

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

Jump to bottom

Fix templated default/example values in config ref docs #16442

Merged
merged 1 commit into from Jun 15, 2021
Merged

Fix templated default/example values in config ref docs #16442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion airflow/configuration.py
View file
Open in desktop
Expand Up @@ -92,7 +92,7 @@ def _default_config_file_path(file_name: str):
return os.path.join(templates_dir, file_name)


def default_config_yaml() -> dict:
def default_config_yaml() -> List[dict]:
Copy link
Member

Choose a reason for hiding this comment

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

Is this right? PyYAML documentation says safe_load only loads the first document, which would be dict.

Copy link
Member

Choose a reason for hiding this comment

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

https://pyyaml.org/wiki/PyYAMLDocumentation

safe_load(stream) parses the given stream and returns a Python object constructed from for the first document in the stream. If there are no documents in the stream, it returns None. safe_load recognizes only standard YAML tags and cannot construct an arbitrary Python object.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it is a list:
https://github.com/apache/airflow/blob/main/airflow/config_templates/config.yml

PyYAML is referring to YAML documents: https://pyyaml.org/wiki/PyYAMLDocumentation#documents

We only have one, but you could have more than one in the steam, e.g:

---
- a:b
---
- c:d

Interestingly, for me safe_load actually raises when it sees more than 1 document 🤷‍♂️

"""
Read Airflow configs from YAML file

Expand Down
14 changes: 13 additions & 1 deletion docs/conf.py
View file
Open in desktop
Expand Up @@ -349,8 +349,20 @@ def _get_rst_filepath_from_path(filepath: str):
) in AirflowConfigParser.deprecated_options.items():
deprecated_options[deprecated_section][deprecated_key] = section, key, since_version

configs = default_config_yaml()

# We want the default/example we show in the docs to reflect the value _after_
# the config has been templated, not before
# e.g. {{dag_id}} in default_config.cfg -> {dag_id} in airflow.cfg, and what we want in docs
keys_to_format = ["default", "example"]
for conf_section in configs:
for option in conf_section["options"]:
for key in keys_to_format:
if option[key] and "{{" in option[key]:
option[key] = option[key].replace("{{", "{").replace("}}", "}")

jinja_contexts = {
'config_ctx': {"configs": default_config_yaml(), "deprecated_options": deprecated_options},
'config_ctx': {"configs": configs, "deprecated_options": deprecated_options},
'quick_start_ctx': {
'doc_root_url': f'https://airflow.apache.org/docs/apache-airflow/{PACKAGE_VERSION}/'
if FOR_PRODUCTION
Expand Down