You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/06/14 22:28:53 UTC
[GitHub] [airflow] jedcunningham opened a new pull request #16442: Fix templated default/example values in config ref docs
jedcunningham opened a new pull request #16442:
URL: https://github.com/apache/airflow/pull/16442
We should show the actual default/example value in the configuration
reference docs, not the templated values.
e.g. `{dag_id}` like you get in a generated airflow.cfg, not `{{dag_id}}
like is stored in the airflow.cfg template.
Before:
<img width="526" alt="Screen Shot 2021-06-14 at 4 11 36 PM" src="https://user-images.githubusercontent.com/66968678/121966903-aa0a7800-cd2c-11eb-9ed9-58b682a2419f.png">
After:
<img width="458" alt="Screen Shot 2021-06-14 at 4 11 45 PM" src="https://user-images.githubusercontent.com/66968678/121966922-b1ca1c80-cd2c-11eb-8c9a-70a4936cf0e1.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #16442: Fix templated default/example values in config ref docs
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #16442:
URL: https://github.com/apache/airflow/pull/16442#discussion_r651948311
##########
File path: airflow/configuration.py
##########
@@ -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]:
Review comment:
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 🤷♂️
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #16442: Fix templated default/example values in config ref docs
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #16442:
URL: https://github.com/apache/airflow/pull/16442#discussion_r651705140
##########
File path: airflow/configuration.py
##########
@@ -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]:
Review comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #16442: Fix templated default/example values in config ref docs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16442:
URL: https://github.com/apache/airflow/pull/16442#issuecomment-861037921
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 main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #16442: Fix templated default/example values in config ref docs
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #16442:
URL: https://github.com/apache/airflow/pull/16442#discussion_r651704590
##########
File path: airflow/configuration.py
##########
@@ -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]:
Review comment:
Is this right? PyYAML documentation says `safe_load` only loads the first document, which would be `dict`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #16442: Fix templated default/example values in config ref docs
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #16442:
URL: https://github.com/apache/airflow/pull/16442
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org