AchimGaedkeLynker commented on code in PR #36828:
URL: https://github.com/apache/airflow/pull/36828#discussion_r1456505130
##########
airflow/providers/amazon/aws/operators/ec2.py:
##########
@@ -183,22 +183,36 @@ def execute(self, context: Context):
MaxCount=self.max_count,
**self.config,
)["Instances"]
- instance_ids = []
- for instance in instances:
- instance_ids.append(instance["InstanceId"])
- self.log.info("Created EC2 instance %s", instance["InstanceId"])
+
+ instance_ids = self._on_kill_instance_ids = [instance["InstanceId"]
for instance in instances]
Review Comment:
This type of clean up in case of an error is very similar to the purpose of
[context/with statements in
python](https://realpython.com/python-with-statement/#the-with-statement-approach)
`on_kill` is called outside the normal execution order as part of the signal
handling for `SIGTERM` - hence the extra variable only for that purpose.
Yes, one could call it `self.instance_ids` and I considered it - this
depends on the lifecycle of the TaskInstance object, call order of the methods.
So the list of instances to clean up should be cleared when XCom has
communicated the result. I named the variable very purpose specific.
Would you think it would be necessary to delete the `_on_kill_instance_ids`,
e.g. as part of the `post_execute` method?
--
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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
