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/12/22 11:33:41 UTC
[GitHub] [airflow] ashb opened a new pull request, #28534: Ensure that pod_mutation_hook is called before logging the pod name
ashb opened a new pull request, #28534:
URL: https://github.com/apache/airflow/pull/28534
Otherwise if pod mutation hook changes the name the log message from the
KubeExecutor would be incorrect. As somewhat of a drive-by-refactor I
have moved the application of the pod_mutation_hook to inside
PodGenerator.construct_pod.
I think the default for `with_mutation_hook` _should_ be True, but it's
not clear to me if anything out side of the Airflow core might ever call
these, and I don't want to apply the hook multiple times, so I have left
the default as `False`.
--
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] ashb merged pull request #28534: Ensure that pod_mutation_hook is called before logging the pod name
Posted by GitBox <gi...@apache.org>.
ashb merged PR #28534:
URL: https://github.com/apache/airflow/pull/28534
--
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] ashb commented on a diff in pull request #28534: Ensure that pod_mutation_hook is called before logging the pod name
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #28534:
URL: https://github.com/apache/airflow/pull/28534#discussion_r1055897468
##########
airflow/kubernetes/pod_generator.py:
##########
@@ -349,6 +354,7 @@ def construct_pod(
scheduler_job_id: str,
run_id: str | None = None,
map_index: int = -1,
+ with_mutation_hook: bool = False,
Review Comment:
```suggestion
*,
with_mutation_hook: bool = False,
```
--
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] ashb commented on a diff in pull request #28534: Ensure that pod_mutation_hook is called before logging the pod name
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #28534:
URL: https://github.com/apache/airflow/pull/28534#discussion_r1055897338
##########
airflow/executors/kubernetes_executor.py:
##########
@@ -346,6 +340,8 @@ def run_next(self, next_job: KubernetesJobType) -> None:
args=command,
pod_override_object=kube_executor_config,
base_worker_pod=base_worker_pod,
+ *,
Review Comment:
```suggestion```
##########
airflow/executors/kubernetes_executor.py:
##########
@@ -346,6 +340,8 @@ def run_next(self, next_job: KubernetesJobType) -> None:
args=command,
pod_override_object=kube_executor_config,
base_worker_pod=base_worker_pod,
+ *,
Review Comment:
```suggestion
```
--
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] dstandish commented on pull request #28534: Ensure that pod_mutation_hook is called before logging the pod name
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #28534:
URL: https://github.com/apache/airflow/pull/28534#issuecomment-1363182640
taking a look, want to sure that pod mutation hook is the last thing that changes the pod...
--
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] dstandish commented on pull request #28534: Ensure that pod_mutation_hook is called before logging the pod name
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #28534:
URL: https://github.com/apache/airflow/pull/28534#issuecomment-1363184101
wow this may be record for most approved 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] ephraimbuddy commented on pull request #28534: Ensure that pod_mutation_hook is called before logging the pod name
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on PR #28534:
URL: https://github.com/apache/airflow/pull/28534#issuecomment-1379931032
> Marking this as a "bug fix" and putting it in for 2.5.1
Moving this to 2.6.0 due to multiple conflicts
--
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] ashb commented on pull request #28534: Ensure that pod_mutation_hook is called before logging the pod name
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #28534:
URL: https://github.com/apache/airflow/pull/28534#issuecomment-1363776954
Marking this as a "bug fix" and putting it in for 2.5.1
--
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] dstandish commented on a diff in pull request #28534: Ensure that pod_mutation_hook is called before logging the pod name
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #28534:
URL: https://github.com/apache/airflow/pull/28534#discussion_r1055721343
##########
airflow/executors/kubernetes_executor.py:
##########
@@ -346,6 +340,7 @@ def run_next(self, next_job: KubernetesJobType) -> None:
args=command,
pod_override_object=kube_executor_config,
base_worker_pod=base_worker_pod,
+ with_mutation_hook=True,
Review Comment:
```suggestion
*,
with_mutation_hook=True,
```
--
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] kaxil commented on pull request #28534: Ensure that pod_mutation_hook is called before logging the pod name
Posted by GitBox <gi...@apache.org>.
kaxil commented on PR #28534:
URL: https://github.com/apache/airflow/pull/28534#issuecomment-1362778781
Test failure: https://github.com/apache/airflow/actions/runs/3757402932/jobs/6384609678#step:6:6853
```
=========================== short test summary info ============================
FAILED tests/executors/test_kubernetes_executor.py::TestKubernetesExecutor::test_run_next_pmh_error
= 1 failed, 2169 passed, 25 skipped, 1 xfailed, 173 warnings in 449.28s (0:07:29) =```
--
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 pull request #28534: Ensure that pod_mutation_hook is called before logging the pod name
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28534:
URL: https://github.com/apache/airflow/pull/28534#issuecomment-1363295309
> wow this may be record for most approved PR :)
Let me approve it again to make it closer to record.
--
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