You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "vandonr-amz (via GitHub)" <gi...@apache.org> on 2023/06/02 22:00:32 UTC
[GitHub] [airflow] vandonr-amz opened a new pull request, #31694: fix return values on glue operators deferrable mode
vandonr-amz opened a new pull request, #31694:
URL: https://github.com/apache/airflow/pull/31694
when implementing the glue deferrable operators in #30948 I forgot to match the return value of the `execute` function in the deferrable handler.
Fixing that here, so that the behavior stays the same when using the deferrable version.
Also changing error handling to throw right away instead of returning the error in an event, saving unnecessary scheduling work.
--
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] pankajkoti commented on a diff in pull request #31694: fix return values on glue operators deferrable mode
Posted by "pankajkoti (via GitHub)" <gi...@apache.org>.
pankajkoti commented on code in PR #31694:
URL: https://github.com/apache/airflow/pull/31694#discussion_r1216720304
##########
airflow/providers/amazon/aws/triggers/glue.py:
##########
@@ -59,5 +59,5 @@ def serialize(self) -> tuple[str, dict[str, Any]]:
async def run(self) -> AsyncIterator[TriggerEvent]:
hook = GlueJobHook(aws_conn_id=self.aws_conn_id)
- await hook.async_job_completion(self.job_name, self.run_id, self.verbose)
- yield TriggerEvent({"status": "success", "message": "Job done"})
+ glue_job_run = await hook.async_job_completion(self.job_name, self.run_id, self.verbose)
+ yield TriggerEvent({"status": "success", "message": "Job done", "value": glue_job_run["JobRunId"]})
Review Comment:
Isn't `glue_job_run["JobRunId"]` the same as `self.run_id`?
Perhaps we could just return that, so no need to fetch the value from the dictionary key.
Upto if you'd like to consider the 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] o-nikolas merged pull request #31694: fix return values on glue operators deferrable mode
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas merged PR #31694:
URL: https://github.com/apache/airflow/pull/31694
--
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] pankajkoti commented on a diff in pull request #31694: fix return values on glue operators deferrable mode
Posted by "pankajkoti (via GitHub)" <gi...@apache.org>.
pankajkoti commented on code in PR #31694:
URL: https://github.com/apache/airflow/pull/31694#discussion_r1216720304
##########
airflow/providers/amazon/aws/triggers/glue.py:
##########
@@ -59,5 +59,5 @@ def serialize(self) -> tuple[str, dict[str, Any]]:
async def run(self) -> AsyncIterator[TriggerEvent]:
hook = GlueJobHook(aws_conn_id=self.aws_conn_id)
- await hook.async_job_completion(self.job_name, self.run_id, self.verbose)
- yield TriggerEvent({"status": "success", "message": "Job done"})
+ glue_job_run = await hook.async_job_completion(self.job_name, self.run_id, self.verbose)
+ yield TriggerEvent({"status": "success", "message": "Job done", "value": glue_job_run["JobRunId"]})
Review Comment:
Isn't the `glue_job_run["JobRunId"]` the same as `self.run_id`?
Perhaps we could just return that, so no need to fetch the value from the dictionary key.
Upto if you'd like to consider the 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] vandonr-amz commented on a diff in pull request #31694: fix return values on glue operators deferrable mode
Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #31694:
URL: https://github.com/apache/airflow/pull/31694#discussion_r1218334776
##########
airflow/providers/amazon/aws/triggers/glue.py:
##########
@@ -59,5 +59,5 @@ def serialize(self) -> tuple[str, dict[str, Any]]:
async def run(self) -> AsyncIterator[TriggerEvent]:
hook = GlueJobHook(aws_conn_id=self.aws_conn_id)
- await hook.async_job_completion(self.job_name, self.run_id, self.verbose)
- yield TriggerEvent({"status": "success", "message": "Job done"})
+ glue_job_run = await hook.async_job_completion(self.job_name, self.run_id, self.verbose)
+ yield TriggerEvent({"status": "success", "message": "Job done", "value": glue_job_run["JobRunId"]})
Review Comment:
hmm yes you're right :p
--
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