You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "syedahsn (via GitHub)" <gi...@apache.org> on 2023/07/11 23:53:24 UTC

[GitHub] [airflow] syedahsn opened a new pull request, #32534: Add Deferrable mode for EMR Serverless Start Job Operator

syedahsn opened a new pull request, #32534:
URL: https://github.com/apache/airflow/pull/32534

   This PR adds the deferrable mode option to `EmrServerlessStartJobOperator`. This PR requires the `EmrServerlessStartApplicationTrigger` which is introduced in #32513 , so that needs to be merged before this PR. I will rebase once #32513 is merged. But in the meantime, this PR can be reviewed.
   
   <!--
    Licensed to the Apache Software Foundation (ASF) under one
    or more contributor license agreements.  See the NOTICE file
    distributed with this work for additional information
    regarding copyright ownership.  The ASF licenses this file
    to you under the Apache License, Version 2.0 (the
    "License"); you may not use this file except in compliance
    with the License.  You may obtain a copy of the License at
   
      http://www.apache.org/licenses/LICENSE-2.0
   
    Unless required by applicable law or agreed to in writing,
    software distributed under the License is distributed on an
    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    KIND, either express or implied.  See the License for the
    specific language governing permissions and limitations
    under the License.
    -->
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of an existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
   


-- 
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] vandonr-amz commented on a diff in pull request #32534: Add Deferrable mode for EMR Serverless Start Job Operator

Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #32534:
URL: https://github.com/apache/airflow/pull/32534#discussion_r1267353688


##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -1227,6 +1256,45 @@ def execute(self, context: Context) -> str | None:
 
         return self.job_id
 
+    def start_job_run_after_defer(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] != "success":
+            self.log.info("Application %s failed to start", self.application_id)
+        response = self.hook.conn.start_job_run(
+            clientToken=self.client_request_token,
+            applicationId=self.application_id,
+            executionRoleArn=self.execution_role_arn,
+            jobDriver=self.job_driver,
+            configurationOverrides=self.configuration_overrides,
+            name=self.name,
+            **self.config,
+        )
+
+        if response["ResponseMetadata"]["HTTPStatusCode"] != 200:
+            raise AirflowException(f"EMR serverless job failed to start: {response}")
+
+        self.defer(
+            trigger=EmrServerlessStartJobTrigger(
+                application_id=self.application_id,
+                job_id=response["jobRunId"],
+                waiter_delay=self.waiter_delay,
+                waiter_max_attempts=self.waiter_max_attempts,
+                aws_conn_id=self.aws_conn_id,
+            ),
+            method_name="execute_complete",
+            timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay),
+        )
+
+    def execute_complete(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] == "success":
+            self.log.info("Serverless job completed")
+            return event["job_id"]
+
     def on_kill(self) -> None:

Review Comment:
   little remark on this, it could be worth it to mention in the doc that the job is stopped if the operator is killed, EXCEPT if deferred.



##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -1227,6 +1256,45 @@ def execute(self, context: Context) -> str | None:
 
         return self.job_id
 
+    def start_job_run_after_defer(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] != "success":
+            self.log.info("Application %s failed to start", self.application_id)
+        response = self.hook.conn.start_job_run(
+            clientToken=self.client_request_token,
+            applicationId=self.application_id,
+            executionRoleArn=self.execution_role_arn,
+            jobDriver=self.job_driver,
+            configurationOverrides=self.configuration_overrides,
+            name=self.name,
+            **self.config,
+        )
+
+        if response["ResponseMetadata"]["HTTPStatusCode"] != 200:
+            raise AirflowException(f"EMR serverless job failed to start: {response}")
+
+        self.defer(
+            trigger=EmrServerlessStartJobTrigger(
+                application_id=self.application_id,
+                job_id=response["jobRunId"],
+                waiter_delay=self.waiter_delay,
+                waiter_max_attempts=self.waiter_max_attempts,
+                aws_conn_id=self.aws_conn_id,
+            ),
+            method_name="execute_complete",
+            timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay),
+        )
+
+    def execute_complete(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] == "success":
+            self.log.info("Serverless job completed")
+            return event["job_id"]

Review Comment:
   I think that discussion has already taken place in a different PR maybe ? But we ignore events where status != success, maybe we'd want to raise or at least log something if that happens ?



-- 
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 merged pull request #32534: Add Deferrable mode for EMR Serverless Start Job Operator

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #32534:
URL: https://github.com/apache/airflow/pull/32534


-- 
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] vandonr-amz commented on a diff in pull request #32534: Add Deferrable mode for EMR Serverless Start Job Operator

Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #32534:
URL: https://github.com/apache/airflow/pull/32534#discussion_r1268555763


##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -1227,6 +1256,45 @@ def execute(self, context: Context) -> str | None:
 
         return self.job_id
 
+    def start_job_run_after_defer(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] != "success":
+            self.log.info("Application %s failed to start", self.application_id)
+        response = self.hook.conn.start_job_run(
+            clientToken=self.client_request_token,
+            applicationId=self.application_id,
+            executionRoleArn=self.execution_role_arn,
+            jobDriver=self.job_driver,
+            configurationOverrides=self.configuration_overrides,
+            name=self.name,
+            **self.config,
+        )
+
+        if response["ResponseMetadata"]["HTTPStatusCode"] != 200:
+            raise AirflowException(f"EMR serverless job failed to start: {response}")
+
+        self.defer(
+            trigger=EmrServerlessStartJobTrigger(
+                application_id=self.application_id,
+                job_id=response["jobRunId"],
+                waiter_delay=self.waiter_delay,
+                waiter_max_attempts=self.waiter_max_attempts,
+                aws_conn_id=self.aws_conn_id,
+            ),
+            method_name="execute_complete",
+            timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay),
+        )
+
+    def execute_complete(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] == "success":
+            self.log.info("Serverless job completed")
+            return event["job_id"]
+
     def on_kill(self) -> None:

Review Comment:
   it's the operator's on_kill method, not the executor. And as soon as you deffer, you're not in the operator anymore, so this won't get called.



-- 
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] vandonr-amz commented on a diff in pull request #32534: Add Deferrable mode for EMR Serverless Start Job Operator

Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #32534:
URL: https://github.com/apache/airflow/pull/32534#discussion_r1268555763


##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -1227,6 +1256,45 @@ def execute(self, context: Context) -> str | None:
 
         return self.job_id
 
+    def start_job_run_after_defer(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] != "success":
+            self.log.info("Application %s failed to start", self.application_id)
+        response = self.hook.conn.start_job_run(
+            clientToken=self.client_request_token,
+            applicationId=self.application_id,
+            executionRoleArn=self.execution_role_arn,
+            jobDriver=self.job_driver,
+            configurationOverrides=self.configuration_overrides,
+            name=self.name,
+            **self.config,
+        )
+
+        if response["ResponseMetadata"]["HTTPStatusCode"] != 200:
+            raise AirflowException(f"EMR serverless job failed to start: {response}")
+
+        self.defer(
+            trigger=EmrServerlessStartJobTrigger(
+                application_id=self.application_id,
+                job_id=response["jobRunId"],
+                waiter_delay=self.waiter_delay,
+                waiter_max_attempts=self.waiter_max_attempts,
+                aws_conn_id=self.aws_conn_id,
+            ),
+            method_name="execute_complete",
+            timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay),
+        )
+
+    def execute_complete(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] == "success":
+            self.log.info("Serverless job completed")
+            return event["job_id"]
+
     def on_kill(self) -> None:

Review Comment:
   it's the operator's on_kill method, not the executor. And as soon as you deffer, you're not in the operator anymore (you're in the trigger), so this won't get called.



-- 
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] vandonr-amz commented on a diff in pull request #32534: Add Deferrable mode for EMR Serverless Start Job Operator

Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #32534:
URL: https://github.com/apache/airflow/pull/32534#discussion_r1268554818


##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -1227,6 +1256,45 @@ def execute(self, context: Context) -> str | None:
 
         return self.job_id
 
+    def start_job_run_after_defer(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] != "success":
+            self.log.info("Application %s failed to start", self.application_id)
+        response = self.hook.conn.start_job_run(
+            clientToken=self.client_request_token,
+            applicationId=self.application_id,
+            executionRoleArn=self.execution_role_arn,
+            jobDriver=self.job_driver,
+            configurationOverrides=self.configuration_overrides,
+            name=self.name,
+            **self.config,
+        )
+
+        if response["ResponseMetadata"]["HTTPStatusCode"] != 200:
+            raise AirflowException(f"EMR serverless job failed to start: {response}")
+
+        self.defer(
+            trigger=EmrServerlessStartJobTrigger(
+                application_id=self.application_id,
+                job_id=response["jobRunId"],
+                waiter_delay=self.waiter_delay,
+                waiter_max_attempts=self.waiter_max_attempts,
+                aws_conn_id=self.aws_conn_id,
+            ),
+            method_name="execute_complete",
+            timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay),
+        )
+
+    def execute_complete(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] == "success":
+            self.log.info("Serverless job completed")
+            return event["job_id"]

Review Comment:
   ok, I see it a bit as "validation" that we didn't get crap data for whatever reason, but you are also right.



-- 
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] syedahsn commented on a diff in pull request #32534: Add Deferrable mode for EMR Serverless Start Job Operator

Posted by "syedahsn (via GitHub)" <gi...@apache.org>.
syedahsn commented on code in PR #32534:
URL: https://github.com/apache/airflow/pull/32534#discussion_r1268332186


##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -1227,6 +1256,45 @@ def execute(self, context: Context) -> str | None:
 
         return self.job_id
 
+    def start_job_run_after_defer(self, context: Context, event: dict[str, Any] | None = None) -> None:

Review Comment:
   That's a really good point, I totally missed that in the documentation. It turns out I didn't need to check for `event["status"]` at all, because there is no need for it. The code follows the correct path because of this statement `if app_state not in EmrServerlessHook.APPLICATION_SUCCESS_STATES:`



-- 
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] Lee-W commented on a diff in pull request #32534: Add Deferrable mode for EMR Serverless Start Job Operator

Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #32534:
URL: https://github.com/apache/airflow/pull/32534#discussion_r1265499110


##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -1227,6 +1256,39 @@ def execute(self, context: Context) -> str | None:
 
         return self.job_id
 
+    def start_job_run_after_defer(self, context: Context, event: dict[str, Any] = {}) -> None:

Review Comment:
   It's suggested not to use mutable objects such as `{}` as the default value.



##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -1227,6 +1256,39 @@ def execute(self, context: Context) -> str | None:
 
         return self.job_id
 
+    def start_job_run_after_defer(self, context: Context, event: dict[str, Any] = {}) -> None:
+        if event["status"] != "success":
+            self.log.info("Application %s failed to start", self.application_id)
+        response = self.hook.conn.start_job_run(
+            clientToken=self.client_request_token,
+            applicationId=self.application_id,
+            executionRoleArn=self.execution_role_arn,
+            jobDriver=self.job_driver,
+            configurationOverrides=self.configuration_overrides,
+            name=self.name,
+            **self.config,
+        )
+
+        if response["ResponseMetadata"]["HTTPStatusCode"] != 200:
+            raise AirflowException(f"EMR serverless job failed to start: {response}")
+
+        self.defer(
+            trigger=EmrServerlessStartJobTrigger(
+                application_id=self.application_id,
+                job_id=response["jobRunId"],
+                waiter_delay=self.waiter_delay,
+                waiter_max_attempts=self.waiter_max_attempts,
+                aws_conn_id=self.aws_conn_id,
+            ),
+            method_name="execute_complete",
+            timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay),
+        )
+
+    def execute_complete(self, context: Context, event: dict[str, Any] = {}) -> None:

Review Comment:
   
   
   It's suggested not to use mutable objects such as {} as the default value.
   



-- 
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] syedahsn commented on a diff in pull request #32534: Add Deferrable mode for EMR Serverless Start Job Operator

Posted by "syedahsn (via GitHub)" <gi...@apache.org>.
syedahsn commented on code in PR #32534:
URL: https://github.com/apache/airflow/pull/32534#discussion_r1268336471


##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -1227,6 +1256,45 @@ def execute(self, context: Context) -> str | None:
 
         return self.job_id
 
+    def start_job_run_after_defer(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] != "success":
+            self.log.info("Application %s failed to start", self.application_id)
+        response = self.hook.conn.start_job_run(
+            clientToken=self.client_request_token,
+            applicationId=self.application_id,
+            executionRoleArn=self.execution_role_arn,
+            jobDriver=self.job_driver,
+            configurationOverrides=self.configuration_overrides,
+            name=self.name,
+            **self.config,
+        )
+
+        if response["ResponseMetadata"]["HTTPStatusCode"] != 200:
+            raise AirflowException(f"EMR serverless job failed to start: {response}")
+
+        self.defer(
+            trigger=EmrServerlessStartJobTrigger(
+                application_id=self.application_id,
+                job_id=response["jobRunId"],
+                waiter_delay=self.waiter_delay,
+                waiter_max_attempts=self.waiter_max_attempts,
+                aws_conn_id=self.aws_conn_id,
+            ),
+            method_name="execute_complete",
+            timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay),
+        )
+
+    def execute_complete(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] == "success":
+            self.log.info("Serverless job completed")
+            return event["job_id"]

Review Comment:
   I addressed this in another [comment ](https://github.com/apache/airflow/pull/32513#discussion_r1265494594). Basically, my point was that the only thing setting `event["status"]` is the Trigger, and it does not set any state beside `success` so it should never be set to anything else. I'm testing for the `success` state just so the code is easier to understand without having to look at the Trigger code.



-- 
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] vandonr-amz commented on a diff in pull request #32534: Add Deferrable mode for EMR Serverless Start Job Operator

Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #32534:
URL: https://github.com/apache/airflow/pull/32534#discussion_r1267352246


##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -1227,6 +1256,45 @@ def execute(self, context: Context) -> str | None:
 
         return self.job_id
 
+    def start_job_run_after_defer(self, context: Context, event: dict[str, Any] | None = None) -> None:

Review Comment:
   I've seen in the docs that you can do a re-entering execute if you add `event` as a parameter, I'm wondering if it would make sense here.
   I'd save some code duplication, with very little `if else` needed I think (since you can rely on the `if app_state not in EmrServerlessHook.APPLICATION_SUCCESS_STATES:` that's already there for the code to take the correct path.
   Just a little check on `event["status"] == "success"` at the top would suffice.



-- 
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] syedahsn commented on a diff in pull request #32534: Add Deferrable mode for EMR Serverless Start Job Operator

Posted by "syedahsn (via GitHub)" <gi...@apache.org>.
syedahsn commented on code in PR #32534:
URL: https://github.com/apache/airflow/pull/32534#discussion_r1268426707


##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -1227,6 +1256,45 @@ def execute(self, context: Context) -> str | None:
 
         return self.job_id
 
+    def start_job_run_after_defer(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] != "success":
+            self.log.info("Application %s failed to start", self.application_id)
+        response = self.hook.conn.start_job_run(
+            clientToken=self.client_request_token,
+            applicationId=self.application_id,
+            executionRoleArn=self.execution_role_arn,
+            jobDriver=self.job_driver,
+            configurationOverrides=self.configuration_overrides,
+            name=self.name,
+            **self.config,
+        )
+
+        if response["ResponseMetadata"]["HTTPStatusCode"] != 200:
+            raise AirflowException(f"EMR serverless job failed to start: {response}")
+
+        self.defer(
+            trigger=EmrServerlessStartJobTrigger(
+                application_id=self.application_id,
+                job_id=response["jobRunId"],
+                waiter_delay=self.waiter_delay,
+                waiter_max_attempts=self.waiter_max_attempts,
+                aws_conn_id=self.aws_conn_id,
+            ),
+            method_name="execute_complete",
+            timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay),
+        )
+
+    def execute_complete(self, context: Context, event: dict[str, Any] | None = None) -> None:
+        if event is None:
+            self.log.error("Trigger error: event is None")
+            raise AirflowException("Trigger error: event is None")
+        elif event["status"] == "success":
+            self.log.info("Serverless job completed")
+            return event["job_id"]
+
     def on_kill(self) -> None:

Review Comment:
   Why would the behaviour be different in the deferred case? Even in the deferrable case, everything is happening from the executor, so if the executor is killed, the `on_kill` method should still be run



-- 
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