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 2020/02/12 06:50:48 UTC
[GitHub] [airflow] brandonwillard opened a new pull request #7405:
[AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
brandonwillard opened a new pull request #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405
This PR adds an option, `kubernetes.worker_pod_uses_args`, that makes `AirflowKubernetesScheduler.run_next` use the Kubernetes `args` field instead of `command` in the worker pod spec.
---
Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
<sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
---
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7405:
[AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405#discussion_r402755433
##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -397,24 +397,32 @@ def reconcile_containers(base_containers: List[k8s.V1Container],
)
@staticmethod
- def construct_pod(
+ def construct_pod( # pylint: disable=too-many-arguments
dag_id: str,
task_id: str,
pod_id: str,
try_number: int,
date: str,
- command: List[str],
+ command: Optional[List[str]],
kube_executor_config: Optional[k8s.V1Pod],
worker_config: k8s.V1Pod,
namespace: str,
- worker_uuid: str
+ worker_uuid: str,
+ use_args: Optional[bool] = False,
) -> k8s.V1Pod:
"""
Construct a pod by gathering and consolidating the configuration from 3 places:
- airflow.cfg
- executor_config
- dynamic arguments
"""
+
+ if use_args:
+ args = command
+ command = []
+ else:
+ args = []
Review comment:
It seems to me useful only for the Executor. What do you think about the constructor_pod method taking parameters args and command, and the main logic being in Kubernetes Executor?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io commented on issue #7405: [AIRFLOW-6780] Add
option to use args instead of command in K8s pod spec
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405#issuecomment-603534537
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=h1) Report
> Merging [#7405](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/1982c3fdca1f04cfc41fc5b5e285d8f01c6b76ab&el=desc) will **decrease** coverage by `28.19%`.
> The diff coverage is `16.66%`.
[](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7405 +/- ##
===========================================
- Coverage 86.97% 58.78% -28.20%
===========================================
Files 927 927
Lines 44963 44968 +5
===========================================
- Hits 39108 26434 -12674
- Misses 5855 18534 +12679
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/kubernetes\_executor.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMva3ViZXJuZXRlc19leGVjdXRvci5weQ==) | `0.00% <0.00%> (-56.88%)` | :arrow_down: |
| [airflow/kubernetes/pod\_generator.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9nZW5lcmF0b3IucHk=) | `20.00% <20.00%> (-76.52%)` | :arrow_down: |
| [airflow/www/forms.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvZm9ybXMucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/www/widgets.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvd2lkZ2V0cy5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/S3\_hook.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9TM19ob29rLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/pig\_hook.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9waWdfaG9vay5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/sensors/python.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZW5zb3JzL3B5dGhvbi5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/utils/asciiart.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9hc2NpaWFydC5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/www/blueprints.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYmx1ZXByaW50cy5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/www/validators.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmFsaWRhdG9ycy5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [513 more](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=footer). Last update [1982c3f...80ab7a8](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] brandonwillard commented on issue #7405: [AIRFLOW-6780]
Add option to use args instead of command in K8s pod spec
Posted by GitBox <gi...@apache.org>.
brandonwillard commented on issue #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405#issuecomment-585449441
@dimberman, do those Travis failures have anything to do with this change? It looks like something's timing out.
I'm seeing the same errors in https://github.com/apache/airflow/pull/7401, as well.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] brandonwillard commented on a change in pull request
#7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod
spec
Posted by GitBox <gi...@apache.org>.
brandonwillard commented on a change in pull request #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405#discussion_r403589325
##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -397,24 +397,32 @@ def reconcile_containers(base_containers: List[k8s.V1Container],
)
@staticmethod
- def construct_pod(
+ def construct_pod( # pylint: disable=too-many-arguments
dag_id: str,
task_id: str,
pod_id: str,
try_number: int,
date: str,
- command: List[str],
+ command: Optional[List[str]],
kube_executor_config: Optional[k8s.V1Pod],
worker_config: k8s.V1Pod,
namespace: str,
- worker_uuid: str
+ worker_uuid: str,
+ use_args: Optional[bool] = False,
) -> k8s.V1Pod:
"""
Construct a pod by gathering and consolidating the configuration from 3 places:
- airflow.cfg
- executor_config
- dynamic arguments
"""
+
+ if use_args:
+ args = command
+ command = []
+ else:
+ args = []
Review comment:
This functionality was only intended for the Executor, and, yeah, the question about where to put it did come to mind (i.e. between `PodGenerator` or `KubernetesExecutor`).
"Thematically" speaking, the option to choose the `args` or `command` field seemed most appropriate for `PodGenerator.construct_pod`, i.e. a method intended to generally construct pods, and one that already takes a `command` argument. (Side note: I'm admittedly not a fan of the `use_args` "switch", but I didn't want to change an interface used by others, so I opted to simply augment/extend it.)
In general, I didn't want to add pod construction/manipulation logic to `KubernetesExecutor.run_next`; it just didn't seem good to have that kind of stuff spread around the code base.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] brandonwillard closed pull request #7405: [AIRFLOW-6780]
Add option to use args instead of command in K8s pod spec
Posted by GitBox <gi...@apache.org>.
brandonwillard closed pull request #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] brandonwillard commented on issue #7405: [AIRFLOW-6780]
Add option to use args instead of command in K8s pod spec
Posted by GitBox <gi...@apache.org>.
brandonwillard commented on issue #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405#issuecomment-609108556
Sorry, I have to move this to my personal fork: #8146
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7405:
[AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405#issuecomment-603534537
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=h1) Report
> Merging [#7405](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/1982c3fdca1f04cfc41fc5b5e285d8f01c6b76ab&el=desc) will **decrease** coverage by `0.64%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7405 +/- ##
==========================================
- Coverage 86.97% 86.33% -0.65%
==========================================
Files 927 927
Lines 44963 44968 +5
==========================================
- Hits 39108 38821 -287
- Misses 5855 6147 +292
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/kubernetes\_executor.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMva3ViZXJuZXRlc19leGVjdXRvci5weQ==) | `56.96% <100.00%> (+0.08%)` | :arrow_up: |
| [airflow/kubernetes/pod\_generator.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9nZW5lcmF0b3IucHk=) | `96.58% <100.00%> (+0.06%)` | :arrow_up: |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
| [airflow/providers/redis/sensors/redis\_key.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvc2Vuc29ycy9yZWRpc19rZXkucHk=) | `61.53% <0.00%> (-38.47%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=footer). Last update [1982c3f...80ab7a8](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7405:
[AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405#issuecomment-603534537
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=h1) Report
> Merging [#7405](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/1982c3fdca1f04cfc41fc5b5e285d8f01c6b76ab&el=desc) will **decrease** coverage by `0.78%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7405 +/- ##
==========================================
- Coverage 86.97% 86.19% -0.79%
==========================================
Files 927 927
Lines 44963 44968 +5
==========================================
- Hits 39108 38760 -348
- Misses 5855 6208 +353
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/kubernetes\_executor.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMva3ViZXJuZXRlc19leGVjdXRvci5weQ==) | `56.96% <100.00%> (+0.08%)` | :arrow_up: |
| [airflow/kubernetes/pod\_generator.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9nZW5lcmF0b3IucHk=) | `96.58% <100.00%> (+0.06%)` | :arrow_up: |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/api/auth/backend/kerberos\_auth.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvYXV0aC9iYWNrZW5kL2tlcmJlcm9zX2F1dGgucHk=) | `28.16% <0.00%> (-54.93%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
| [airflow/security/kerberos.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZWN1cml0eS9rZXJiZXJvcy5weQ==) | `30.43% <0.00%> (-45.66%)` | :arrow_down: |
| ... and [10 more](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=footer). Last update [1982c3f...80ab7a8](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7405:
[AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405#issuecomment-603534537
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=h1) Report
> Merging [#7405](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/1982c3fdca1f04cfc41fc5b5e285d8f01c6b76ab&el=desc) will **decrease** coverage by `0.89%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7405 +/- ##
==========================================
- Coverage 86.97% 86.08% -0.90%
==========================================
Files 927 927
Lines 44963 44968 +5
==========================================
- Hits 39108 38712 -396
- Misses 5855 6256 +401
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/kubernetes\_executor.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMva3ViZXJuZXRlc19leGVjdXRvci5weQ==) | `56.96% <100.00%> (+0.08%)` | :arrow_up: |
| [airflow/kubernetes/pod\_generator.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9nZW5lcmF0b3IucHk=) | `96.58% <100.00%> (+0.06%)` | :arrow_up: |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
| [airflow/operators/generic\_transfer.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvZ2VuZXJpY190cmFuc2Zlci5weQ==) | `39.28% <0.00%> (-60.72%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/api/auth/backend/kerberos\_auth.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvYXV0aC9iYWNrZW5kL2tlcmJlcm9zX2F1dGgucHk=) | `28.16% <0.00%> (-54.93%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/airflow/pull/7405/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=footer). Last update [1982c3f...80ab7a8](https://codecov.io/gh/apache/airflow/pull/7405?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] brandonwillard commented on a change in pull request
#7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod
spec
Posted by GitBox <gi...@apache.org>.
brandonwillard commented on a change in pull request #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405#discussion_r403589325
##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -397,24 +397,32 @@ def reconcile_containers(base_containers: List[k8s.V1Container],
)
@staticmethod
- def construct_pod(
+ def construct_pod( # pylint: disable=too-many-arguments
dag_id: str,
task_id: str,
pod_id: str,
try_number: int,
date: str,
- command: List[str],
+ command: Optional[List[str]],
kube_executor_config: Optional[k8s.V1Pod],
worker_config: k8s.V1Pod,
namespace: str,
- worker_uuid: str
+ worker_uuid: str,
+ use_args: Optional[bool] = False,
) -> k8s.V1Pod:
"""
Construct a pod by gathering and consolidating the configuration from 3 places:
- airflow.cfg
- executor_config
- dynamic arguments
"""
+
+ if use_args:
+ args = command
+ command = []
+ else:
+ args = []
Review comment:
This functionality was only intended for the Executor, and, yeah, the question about where to put it did come to mind (i.e. between `PodGenerator` or `KubernetesExecutor`).
If I recall, I opted to make the change in the location where the closest/most relevant settings were already being made: . That's where `cmd` was being set.
"Thematically" speaking, the option to choose the `args` or `command` field seemed most appropriate for `PodGenerator.construct_pod`, i.e. a method intended to generally construct pods, and one that already takes a `command` argument. (Side note: I'm admittedly not a fan of the `use_args` "switch", but I didn't want to change an interface used by others, so I opted to simply augment/extend it.)
In general, I didn't want to add pod construction/manipulation logic to `KubernetesExecutor.run_next`; it just didn't seem good to have that kind of stuff spread around the code base.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #7405: [AIRFLOW-6780] Add
option to use args instead of command in K8s pod spec
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7405: [AIRFLOW-6780] Add option to use args instead of command in K8s pod spec
URL: https://github.com/apache/airflow/pull/7405#issuecomment-608246432
Travis is sad. I restarted the failed build, but it seems to me that this could be caused by a change in this 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
With regards,
Apache Git Services