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/05/07 21:47:06 UTC
[GitHub] [airflow] jedcunningham opened a new pull request #15729: Clean up unnecessary Airflow config in helm chart
jedcunningham opened a new pull request #15729:
URL: https://github.com/apache/airflow/pull/15729
This removes Airflow config from the chart that isn't necessary.
The configs are either already the default, don't need to changed from the
default, or aren't real Airflow configs anyways.
--
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 #15729: Clean up unnecessary Airflow config in helm chart
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #15729:
URL: https://github.com/apache/airflow/pull/15729
--
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 #15729: Clean up unnecessary Airflow config in helm chart
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #15729:
URL: https://github.com/apache/airflow/pull/15729#discussion_r629486438
##########
File path: chart/values.yaml
##########
@@ -914,10 +912,6 @@ config:
elasticsearch:
json_format: 'True'
log_id_template: "{dag_id}_{task_id}_{execution_date}_{try_number}"
- elasticsearch_configs:
- max_retries: 3
- timeout: 30
- retry_timeout: 'True'
Review comment:
Thanks. I'll probably document that better in the default airflow config in a follow up PR.
--
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 #15729: Clean up unnecessary Airflow config in helm chart
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15729:
URL: https://github.com/apache/airflow/pull/15729#issuecomment-834850505
[The Workflow run](https://github.com/apache/airflow/actions/runs/821823044) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
--
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 closed pull request #15729: Clean up unnecessary Airflow config in helm chart
Posted by GitBox <gi...@apache.org>.
jedcunningham closed pull request #15729:
URL: https://github.com/apache/airflow/pull/15729
--
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 #15729: Clean up unnecessary Airflow config in helm chart
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #15729:
URL: https://github.com/apache/airflow/pull/15729#discussion_r628553454
##########
File path: chart/values.yaml
##########
@@ -914,10 +912,6 @@ config:
elasticsearch:
json_format: 'True'
log_id_template: "{dag_id}_{task_id}_{execution_date}_{try_number}"
- elasticsearch_configs:
- max_retries: 3
- timeout: 30
- retry_timeout: 'True'
Review comment:
These don't seem to be valid configs in any version? Do they ring a bell?
--
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 commented on a change in pull request #15729: Clean up unnecessary Airflow config in helm chart
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #15729:
URL: https://github.com/apache/airflow/pull/15729#discussion_r629432849
##########
File path: chart/values.yaml
##########
@@ -914,10 +912,6 @@ config:
elasticsearch:
json_format: 'True'
log_id_template: "{dag_id}_{task_id}_{execution_date}_{try_number}"
- elasticsearch_configs:
- max_retries: 3
- timeout: 30
- retry_timeout: 'True'
Review comment:
This is valid:
https://airflow.apache.org/docs/apache-airflow-providers-elasticsearch/stable/logging.html#writing-logs-to-elasticsearch-over-tls
This line: https://github.com/apache/airflow/blob/657384615fafc060f9e2ed925017306705770355/airflow/providers/elasticsearch/log/es_task_handler.py#L76 read all the configs passed to `elasticsearch_configs` and passes them as kwargs to ES client
https://github.com/apache/airflow/blob/657384615fafc060f9e2ed925017306705770355/airflow/providers/elasticsearch/log/es_task_handler.py#L89
--
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 #15729: Clean up unnecessary Airflow config in helm chart
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15729:
URL: https://github.com/apache/airflow/pull/15729#issuecomment-834850783
[The Workflow run](https://github.com/apache/airflow/actions/runs/821847833) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
--
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 commented on a change in pull request #15729: Clean up unnecessary Airflow config in helm chart
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #15729:
URL: https://github.com/apache/airflow/pull/15729#discussion_r629432849
##########
File path: chart/values.yaml
##########
@@ -914,10 +912,6 @@ config:
elasticsearch:
json_format: 'True'
log_id_template: "{dag_id}_{task_id}_{execution_date}_{try_number}"
- elasticsearch_configs:
- max_retries: 3
- timeout: 30
- retry_timeout: 'True'
Review comment:
This is valid:
https://airflow.apache.org/docs/apache-airflow-providers-elasticsearch/stable/logging.html#writing-logs-to-elasticsearch-over-tls
This line: https://github.com/apache/airflow/blob/657384615fafc060f9e2ed925017306705770355/airflow/providers/elasticsearch/log/es_task_handler.py#L76 read all the configs passed to `elasticsearch_configs` and passes them as kwargs to ES client
--
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