vandonr-amz commented on code in PR #31838:
URL: https://github.com/apache/airflow/pull/31838#discussion_r1225658167
##########
tests/system/providers/amazon/aws/example_ecs.py:
##########
@@ -67,6 +66,15 @@ def get_region():
return boto3.session.Session().region_name
+@task(trigger_rule=TriggerRule.ALL_DONE)
+def clean_logs(group_name: str):
+ client = boto3.client("logs")
+ # A bit brutal to delete the whole group, I know,
+ # but we don't have the access to the arn of the task which is used in the
stream name
+ # and also those logs just contain "hello world", which is not very
interesting.
+ client.delete_log_group(logGroupName=group_name)
Review Comment:
this is going to fail if the group does not exist, so in a way it makes sure
the log configuration stays correct.
##########
airflow/providers/amazon/aws/operators/ecs.py:
##########
@@ -480,6 +480,15 @@ def __init__(
self.waiter_delay = waiter_delay
self.waiter_max_attempts = waiter_max_attempts
+ if self._aws_logs_enabled() and not self.wait_for_completion:
+ self.log.warning(
+ "Trying to get logs without waiting for the task to complete
is undefined behavior."
+ )
+
+ @staticmethod
+ def _get_ecs_task_id(task_arn: str) -> str:
+ return task_arn.split("/")[-1]
Review Comment:
doing this because I thought that remembering to update the ecs_task_id
every time the arn gets changed was a bit brittle. This method makes the
dependency arn->task_id explicit.
Yes it means we're going to recompute it each time we need it, but I think
it's negligible.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
