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 2022/09/19 16:24:11 UTC
[GitHub] [airflow] jtvmatos opened a new issue, #26494: [Helm Chart] Support multiple DagProcessors
jtvmatos opened a new issue, #26494:
URL: https://github.com/apache/airflow/issues/26494
### Description
Airflow 2.4.0 has introduced support for [Support multiple DagProcessors parsing files from different locations](https://github.com/apache/airflow/pull/25935).
However, Helm chart only supports [Provision Standalone Dag Processor](https://github.com/apache/airflow/pull/23711) (or at least will support in Airflow Helm Chart 1.7.0).
### Use case/motivation
For Airflow Helm Chart users take advantage of these new [AIP-43](https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-43+DAG+Processor+separation) features, it is necessary to add support for provision multiple DagProcessors in Helm chart.
### Related issues
[Helmchart: Provision Standalone Dag Processor](https://github.com/apache/airflow/pull/23711)
[Support multiple DagProcessors parsing files from different locations](https://github.com/apache/airflow/pull/25935)
### Are you willing to submit a PR?
- [ ] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #26494: [Helm Chart] Support multiple DagProcessors
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #26494:
URL: https://github.com/apache/airflow/issues/26494#issuecomment-1252299098
Marked it as a good first issue. I think it would require some smart way of defining how to do it in the configuration. Even if you are not willling to implement it (or maybe?), a proposal from the user perspective how it could look like when it comes to configuration would be most welcome @jtvmatos . Any ideas and suggestions?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #26494: [Helm Chart] Support multiple DagProcessors
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #26494:
URL: https://github.com/apache/airflow/issues/26494#issuecomment-1254793511
No worries - we do not have to add it super-quickly I think (but if somoene will want to add it this is still an option :)).
I'd say the best approach would be to make the top-level dag processor entry optional (and fail the configuration if both "arrays" and "top level" are specified (then we could name the array "multipleDagProcessors" simply?). WDYT @jedcunningham @mhenc ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #26494: [Helm Chart] Support multiple DagProcessors
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on issue #26494:
URL: https://github.com/apache/airflow/issues/26494#issuecomment-1404993715
@jtvmatos I think it's better to open a draft PR and discuss code changes over the 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jtvmatos commented on issue #26494: [Helm Chart] Support multiple DagProcessors
Posted by GitBox <gi...@apache.org>.
jtvmatos commented on issue #26494:
URL: https://github.com/apache/airflow/issues/26494#issuecomment-1253268272
Hi @potiuk thanks for the feedback.
Unfortunately in the coming weeks I won't be available to create this PR (I will be unavailable for more than a month... if no one solves it by then, I can open the PR....)
But yes, this PR can be tricky and can create a breaking change in the current processor configurations. For example, creating a nested level for multiple DagProcessors could be an option:
```
# Airflow Dag Processor Config
dagProcessor:
enabled: false
# Airflow dag processors in the deployment
processor:
# Processor 1
- name: processor-1 # Alias for metadata.name
replicas: 1
revisionHistoryLimit: ~
command: ~
args: ["bash", "-c", "exec airflow dag-processor"]
strategy:
(...)
# Processor 2
# - name: processor-2
# args: ["bash", "-c", "exec airflow dag-processor --subdir ..."]
```
With this values.yaml, will be possible to set the processors configurations as currently used for `env` or `secrets`: `dagProcessor.processor[0]...`, `dagProcessor.processor[1]...` for N processors (>=2.4.0).
Alternatively, we can just add a `extraDagProcessor` list:
```
# Airflow Dag Processor Config
dagProcessor:
enabled: false
(...)
env: []
extraDagProcessor: []
```
This list is not `breaking` for the current settings. However, it may imply that there is a "master" processor.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jtvmatos commented on issue #26494: [Helm Chart] Support multiple DagProcessors
Posted by GitBox <gi...@apache.org>.
jtvmatos commented on issue #26494:
URL: https://github.com/apache/airflow/issues/26494#issuecomment-1254803364
I create a fork, with the approach that I mention before (only for my local tests):
https://github.com/apache/airflow/commit/0793c55be0f63d8da53d16c1097bc274bbcc1a69
For now this is all I can do. But it can be useful for discussion :)
Thank you all!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org