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 2020/08/29 20:53:49 UTC
[GitHub] [airflow] darwinyip opened a new pull request #10643: Ecs operator reattach
darwinyip opened a new pull request #10643:
URL: https://github.com/apache/airflow/pull/10643
closes #10620
Adds `reattach` flag to `ECSOperator` so that whenever the Airflow server restarts, it does not leave rogue ECS Tasks. Instead the operator will seek for any running instance and attach to it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] feluelle edited a comment on pull request #10643: ECSOperator: reattach flag
Posted by GitBox <gi...@apache.org>.
feluelle edited a comment on pull request #10643:
URL: https://github.com/apache/airflow/pull/10643#issuecomment-714333858
Recently type annotations were added to the aws provider. That's why I would like you to rebase (and fix conflicts) - and if that goes fine I am merging it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] darwinyip commented on pull request #10643: ECSOperator: reattach flag
Posted by GitBox <gi...@apache.org>.
darwinyip commented on pull request #10643:
URL: https://github.com/apache/airflow/pull/10643#issuecomment-706448338
@feluelle Can you recommend how to go about testing this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #10643: ECSOperator: reattach flag
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10643:
URL: https://github.com/apache/airflow/pull/10643#issuecomment-706490045
[The Workflow run](https://github.com/apache/airflow/actions/runs/298739649) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] feluelle merged pull request #10643: Add reattach flag to ECSOperator
Posted by GitBox <gi...@apache.org>.
feluelle merged pull request #10643:
URL: https://github.com/apache/airflow/pull/10643
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] feluelle commented on pull request #10643: ECSOperator: reattach flag
Posted by GitBox <gi...@apache.org>.
feluelle commented on pull request #10643:
URL: https://github.com/apache/airflow/pull/10643#issuecomment-714333858
Recently type annotations were added to the aws provider. That's why I would like you to rebase - and if that goes fine I am merging it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #10643: ECSOperator: reattach flag
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10643:
URL: https://github.com/apache/airflow/pull/10643#issuecomment-706460970
[The Workflow run](https://github.com/apache/airflow/actions/runs/298511806) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] feluelle commented on a change in pull request #10643: ECSOperator: reattach flag
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #10643:
URL: https://github.com/apache/airflow/pull/10643#discussion_r509981714
##########
File path: tests/providers/amazon/aws/operators/test_ecs.py
##########
@@ -269,3 +269,48 @@ def test_check_success_task_not_raises(self):
}
self.ecs._check_success_task()
client_mock.describe_tasks.assert_called_once_with(cluster='c', tasks=['arn'])
+
+ @parameterized.expand(
+ [
+ ['EC2', None],
+ ['FARGATE', None],
+ ['EC2', {'testTagKey': 'testTagValue'}],
+ ['', {'testTagKey': 'testTagValue'}],
+ ]
+ )
+ @mock.patch.object(ECSOperator, '_wait_for_task_ended')
+ @mock.patch.object(ECSOperator, '_check_success_task')
+ @mock.patch.object(ECSOperator, '_start_task')
+ def test_reattach_successful(self, launch_type, tags, start_mock, check_mock, wait_mock):
+
+ self.set_up_operator(launch_type=launch_type, tags=tags) # pylint: disable=no-value-for-parameter
+ client_mock = self.aws_hook_mock.return_value.get_conn.return_value
+ client_mock.describe_task_definition.return_value = {'taskDefinition': {'family': 'f'}}
+ client_mock.list_tasks.return_value = {
+ 'taskArns': ['arn:aws:ecs:us-east-1:012345678910:task/d8c67b3c-ac87-4ffe-a847-4785bc3a8b55']
+ }
+
+ self.ecs.reattach = True
+ self.ecs.execute(None)
+
+ self.aws_hook_mock.return_value.get_conn.assert_called_once()
+ extend_args = {}
+ if launch_type:
+ extend_args['launchType'] = launch_type
+ if launch_type == 'FARGATE':
+ extend_args['platformVersion'] = 'LATEST'
+ if tags:
+ extend_args['tags'] = [{'key': k, 'value': v} for (k, v) in tags.items()]
+
+ client_mock.describe_task_definition.assert_called_once_with('t')
+
+ client_mock.list_tasks.assert_called_once_with(
+ cluster='c', launchType=launch_type, desiredStatus='RUNNING', family='f'
+ )
+
+ start_mock.assert_not_called()
+ wait_mock.assert_called_once_with()
+ check_mock.assert_called_once_with()
+ self.assertEqual(
+ self.ecs.arn, 'arn:aws:ecs:us-east-1:012345678910:task/d8c67b3c-ac87-4ffe-a847-4785bc3a8b55'
+ )
Review comment:
Exactly. That's what I had in mind. Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #10643: ECSOperator: reattach flag
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10643:
URL: https://github.com/apache/airflow/pull/10643#issuecomment-706484192
[The Workflow run](https://github.com/apache/airflow/actions/runs/298679977) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org