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 2022/08/25 23:39:55 UTC

[GitHub] [airflow] syedahsn opened a new pull request, #25971: Athena and EMR operator max_retries mix-up fix (Fix for #22381)

syedahsn opened a new pull request, #25971:
URL: https://github.com/apache/airflow/pull/25971

   Fix for issue regarding a mix up of variable names. The solution proposed changes the variable name `max_retires` to `max_polling_attempts`. Both names are currently accepted, but if `max_tries` is used, the user is shown a deprecation warning. Internally, the operators only use `max_polling_attempts`. If both are passed, but their values are not equal, an Exception is thrown.
   
   closes: [#22381](https://github.com/apache/airflow/issues/22381)
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of an existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
   


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] potiuk commented on pull request #25971: Athena and EMR operator max_retries mix-up fix

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

   Some failures.


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] uranusjr commented on a diff in pull request #25971: Athena and EMR operator max_retries mix-up fix

Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25971:
URL: https://github.com/apache/airflow/pull/25971#discussion_r956775475


##########
airflow/providers/amazon/aws/hooks/athena.py:
##########
@@ -188,17 +189,35 @@ def get_query_results_paginator(
         paginator = self.get_conn().get_paginator('get_query_results')
         return paginator.paginate(**result_params)
 
-    def poll_query_status(self, query_execution_id: str, max_tries: Optional[int] = None) -> Optional[str]:
+    def poll_query_status(
+        self,
+        query_execution_id: str,
+        max_polling_attempts: Optional[int] = None,
+        max_tries: Optional[int] = None,
+    ) -> Optional[str]:
         """
         Poll the status of submitted athena query until query state reaches final state.
         Returns one of the final states
 
         :param query_execution_id: Id of submitted athena query
-        :param max_tries: Number of times to poll for query state before function exits
+        :param max_tries: Deprecated - Use max_polling_attempts instead
+        :param max_polling_attempts: Number of times to poll for query state before function exits
         :return: str
         """
+        if max_tries:
+            warnings.warn(
+                f"Method `{self.__class__.__name__}.max_tries` is deprecated and will be removed "
+                "in a future release.  Please use method `max_polling_attempts` instead.",

Review Comment:
   This message does not read right. [Method](https://en.wikipedia.org/wiki/Method_(computer_programming)) is a function on an instance, but these are not.



-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] potiuk merged pull request #25971: Athena and EMR operator max_retries mix-up fix

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


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] syedahsn commented on a diff in pull request #25971: Athena and EMR operator max_retries mix-up fix

Posted by GitBox <gi...@apache.org>.
syedahsn commented on code in PR #25971:
URL: https://github.com/apache/airflow/pull/25971#discussion_r957511335


##########
airflow/providers/amazon/aws/hooks/athena.py:
##########
@@ -188,17 +189,35 @@ def get_query_results_paginator(
         paginator = self.get_conn().get_paginator('get_query_results')
         return paginator.paginate(**result_params)
 
-    def poll_query_status(self, query_execution_id: str, max_tries: Optional[int] = None) -> Optional[str]:
+    def poll_query_status(
+        self,
+        query_execution_id: str,
+        max_polling_attempts: Optional[int] = None,
+        max_tries: Optional[int] = None,
+    ) -> Optional[str]:
         """
         Poll the status of submitted athena query until query state reaches final state.
         Returns one of the final states
 
         :param query_execution_id: Id of submitted athena query
-        :param max_tries: Number of times to poll for query state before function exits
+        :param max_tries: Deprecated - Use max_polling_attempts instead
+        :param max_polling_attempts: Number of times to poll for query state before function exits
         :return: str
         """
+        if max_tries:
+            warnings.warn(
+                f"Method `{self.__class__.__name__}.max_tries` is deprecated and will be removed "
+                "in a future release.  Please use method `max_polling_attempts` instead.",

Review Comment:
   Missed that, 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org