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/03/08 10:00:50 UTC

[GitHub] [airflow] ashb commented on a change in pull request #14263: Added retry to ECS Operator

ashb commented on a change in pull request #14263:
URL: https://github.com/apache/airflow/pull/14263#discussion_r589292633



##########
File path: airflow/providers/amazon/aws/hooks/base_aws.py
##########
@@ -484,6 +504,33 @@ def expand_role(self, role: str) -> str:
         else:
             return self.get_client_type("iam").get_role(RoleName=role)["Role"]["Arn"]
 
+    @staticmethod
+    def retry(fun: Callable):

Review comment:
       If we change this to
   
   ```python
       def retry(should_retry: Callable[[Exception], bool], fun: Callable)
   ```
   
   Then the `retry_if_permissible_error` can move  out of BaseAWS in to ECSHook, used like this:
   
   
   ```python 
   def should_retry(exception: Exception):
       """Check if exception is related to ECS resource quota (CPU, MEM)."""
       return isinstance(exception, ECSOperatorError) and any(
           quota_reason in failure['reason']
           for quota_reason in ['RESOURCE:MEMORY', 'RESOURCE:CPU']
           for failure in exception.failures
       )
   
   ...
   
       @AwsBaseHook.retry(should_retry)
       def _start_task(self):
   ```

##########
File path: airflow/providers/amazon/aws/hooks/base_aws.py
##########
@@ -484,6 +504,33 @@ def expand_role(self, role: str) -> str:
         else:
             return self.get_client_type("iam").get_role(RoleName=role)["Role"]["Arn"]
 
+    @staticmethod
+    def retry(fun: Callable):
+        """
+        A decorator that provides a mechanism to repeat requests in response to exceeding a temporary quote
+        limit.
+        """
+
+        def decorator_f(self):

Review comment:
       ```suggestion
           @functools.wraps
           def decorator_f(self, *args, **kwargs):
   ```




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