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 2021/05/25 14:51:39 UTC

[GitHub] [airflow] scottypate opened a new pull request #16050: Update Boto3 API calls in ECSOperator

scottypate opened a new pull request #16050:
URL: https://github.com/apache/airflow/pull/16050


   This PR corrects two errors that can occur when utilizing the parameter `reattach=True`. These two errors are detailed below. 
   
   1. The `describe_task_definition()` function requires a named keyword argument instead of a positional argument. The current error generated is...
   
   ```
   [2021-05-25 14:33:24,145] {{taskinstance.py:1455}} ERROR - describe_task_definition() only accepts keyword arguments.
   Traceback (most recent call last):
     File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1112, in _run_raw_task
       self._prepare_and_execute_task_with_callbacks(context, task)
     File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1285, in _prepare_and_execute_task_with_callbacks
       result = self._execute_task(context, task_copy)
     File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1310, in _execute_task
       result = task_copy.execute(context=context)
     File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/amazon/aws/operators/ecs.py", line 193, in execute
       self._try_reattach_task()
     File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/amazon/aws/operators/ecs.py", line 244, in _try_reattach_task
       task_def_resp = self.client.describe_task_definition(self.task_definition)
     File "/home/airflow/.local/lib/python3.8/site-packages/botocore/client.py", line 354, in _api_call
       raise TypeError(
   TypeError: describe_task_definition() only accepts keyword arguments.
   ```
   
   2. The `list_tasks()` function cannot provide both `launch_type` and `family` as a filter criteria. The current error generated is...
   
   ```
   [2021-05-25 12:32:18,441] {{taskinstance.py:1455}} ERROR - An error occurred (InvalidParameterException) when calling the ListTasks operation: cannot specify filters 'launchType' and 'family' at the same time
   Traceback (most recent call last):
     File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1112, in _run_raw_task
       self._prepare_and_execute_task_with_callbacks(context, task)
     File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1285, in _prepare_and_execute_task_with_callbacks
       result = self._execute_task(context, task_copy)
     File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1310, in _execute_task
       result = task_copy.execute(context=context)
     File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/amazon/aws/operators/ecs.py", line 193, in execute
       self._try_reattach_task()
     File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/amazon/aws/operators/ecs.py", line 247, in _try_reattach_task
       list_tasks_resp = self.client.list_tasks(
     File "/home/airflow/.local/lib/python3.8/site-packages/botocore/client.py", line 357, in _api_call
       return self._make_api_call(operation_name, kwargs)
     File "/home/airflow/.local/lib/python3.8/site-packages/botocore/client.py", line 676, in _make_api_call
       raise error_class(parsed_response, operation_name)
   ```
   
   When creating a task definition with the same family name but changing the `launch_type`, then it creates a new revision of the task with the same family name. I removed this `launch_type` parameter from this function. The ECS task will be running the latest version of the task definition and I don't think it would be possible to be running the same family of task definition on multiple `launch_type` strategies. 


-- 
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] boring-cyborg[bot] commented on pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #16050:
URL: https://github.com/apache/airflow/pull/16050#issuecomment-867141108


   Awesome work, congrats on your first merged pull request!
   


-- 
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] boring-cyborg[bot] commented on pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #16050:
URL: https://github.com/apache/airflow/pull/16050#issuecomment-847936305


   Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
   Here are some useful points:
   - Pay attention to the quality of your code (flake8, pylint and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
   - In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/master/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
   - Consider using [Breeze environment](https://github.com/apache/airflow/blob/master/BREEZE.rst) for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
   - Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
   - Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
   - Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#coding-style-and-best-practices).
   Apache Airflow is a community-driven project and together we are making it better πŸš€.
   In case of doubts contact the developers at:
   Mailing List: dev@airflow.apache.org
   Slack: https://s.apache.org/airflow-slack
   


-- 
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] o-nikolas commented on a change in pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
o-nikolas commented on a change in pull request #16050:
URL: https://github.com/apache/airflow/pull/16050#discussion_r655652267



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -266,11 +266,11 @@ def _start_task(self):
         self.arn = response['tasks'][0]['taskArn']
 
     def _try_reattach_task(self):
-        task_def_resp = self.client.describe_task_definition(self.task_definition)
+        task_def_resp = self.client.describe_task_definition(taskDefinition=self.task_definition)
         ecs_task_family = task_def_resp['taskDefinition']['family']
 
         list_tasks_resp = self.client.list_tasks(
-            cluster=self.cluster, launchType=self.launch_type, desiredStatus='RUNNING', family=ecs_task_family
+            cluster=self.cluster, desiredStatus='RUNNING', family=ecs_task_family

Review comment:
       Got it, must have missed that!




-- 
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] potiuk commented on pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16050:
URL: https://github.com/apache/airflow/pull/16050#issuecomment-860201551


   Reopened to rebuild 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] scottypate commented on pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
scottypate commented on pull request #16050:
URL: https://github.com/apache/airflow/pull/16050#issuecomment-847937571


   πŸ‘‹ @darwinyip, since you added the `reattach` option to this operator I wonder if you would look at this PR and see if it makes sense to you. 


-- 
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] potiuk closed pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #16050:
URL: https://github.com/apache/airflow/pull/16050


   


-- 
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] eladkal commented on pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #16050:
URL: https://github.com/apache/airflow/pull/16050#issuecomment-860597239


   @scottypate can you rebase? it will most likely solve the CI build issue


-- 
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] RosterIn commented on a change in pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
RosterIn commented on a change in pull request #16050:
URL: https://github.com/apache/airflow/pull/16050#discussion_r655207710



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -266,11 +266,11 @@ def _start_task(self):
         self.arn = response['tasks'][0]['taskArn']
 
     def _try_reattach_task(self):
-        task_def_resp = self.client.describe_task_definition(self.task_definition)
+        task_def_resp = self.client.describe_task_definition(taskDefinition=self.task_definition)
         ecs_task_family = task_def_resp['taskDefinition']['family']
 
         list_tasks_resp = self.client.list_tasks(
-            cluster=self.cluster, launchType=self.launch_type, desiredStatus='RUNNING', family=ecs_task_family
+            cluster=self.cluster, desiredStatus='RUNNING', family=ecs_task_family

Review comment:
       the op explained it in the PR description. I think it makes sense




-- 
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] potiuk merged pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #16050:
URL: https://github.com/apache/airflow/pull/16050


   


-- 
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] RosterIn commented on a change in pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
RosterIn commented on a change in pull request #16050:
URL: https://github.com/apache/airflow/pull/16050#discussion_r655207710



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -266,11 +266,11 @@ def _start_task(self):
         self.arn = response['tasks'][0]['taskArn']
 
     def _try_reattach_task(self):
-        task_def_resp = self.client.describe_task_definition(self.task_definition)
+        task_def_resp = self.client.describe_task_definition(taskDefinition=self.task_definition)
         ecs_task_family = task_def_resp['taskDefinition']['family']
 
         list_tasks_resp = self.client.list_tasks(
-            cluster=self.cluster, launchType=self.launch_type, desiredStatus='RUNNING', family=ecs_task_family
+            cluster=self.cluster, desiredStatus='RUNNING', family=ecs_task_family

Review comment:
       the op explained it in the PR description. I think it makes sense




-- 
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] o-nikolas commented on a change in pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
o-nikolas commented on a change in pull request #16050:
URL: https://github.com/apache/airflow/pull/16050#discussion_r655652267



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -266,11 +266,11 @@ def _start_task(self):
         self.arn = response['tasks'][0]['taskArn']
 
     def _try_reattach_task(self):
-        task_def_resp = self.client.describe_task_definition(self.task_definition)
+        task_def_resp = self.client.describe_task_definition(taskDefinition=self.task_definition)
         ecs_task_family = task_def_resp['taskDefinition']['family']
 
         list_tasks_resp = self.client.list_tasks(
-            cluster=self.cluster, launchType=self.launch_type, desiredStatus='RUNNING', family=ecs_task_family
+            cluster=self.cluster, desiredStatus='RUNNING', family=ecs_task_family

Review comment:
       Got it, must have missed that!




-- 
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] o-nikolas commented on a change in pull request #16050: Update Boto3 API calls in ECSOperator

Posted by GitBox <gi...@apache.org>.
o-nikolas commented on a change in pull request #16050:
URL: https://github.com/apache/airflow/pull/16050#discussion_r653920886



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -266,11 +266,11 @@ def _start_task(self):
         self.arn = response['tasks'][0]['taskArn']
 
     def _try_reattach_task(self):
-        task_def_resp = self.client.describe_task_definition(self.task_definition)
+        task_def_resp = self.client.describe_task_definition(taskDefinition=self.task_definition)
         ecs_task_family = task_def_resp['taskDefinition']['family']
 
         list_tasks_resp = self.client.list_tasks(
-            cluster=self.cluster, launchType=self.launch_type, desiredStatus='RUNNING', family=ecs_task_family
+            cluster=self.cluster, desiredStatus='RUNNING', family=ecs_task_family

Review comment:
       How did you decide which of the two arguments to drop? 




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