You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ferruzzi (via GitHub)" <gi...@apache.org> on 2023/02/28 21:35:04 UTC
[GitHub] [airflow] ferruzzi opened a new pull request, #29822: Implement custom boto waiters for some EMR operators
ferruzzi opened a new pull request, #29822:
URL: https://github.com/apache/airflow/pull/29822
Part of an ongoing effort to migrate off the `waiter` class in the Amazon Provider Package to the custom boto waiters.
Converts three EMR operators over to new custom waiters and adds a note in the existing Waiter() class to steer people away from it until we can formally deprecate it.
Tagging @Taragolis who is doing related work
AmPP team ping: @o-nikolas @vincbeck @vandonr-amz @syedahsn
--
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] Taragolis commented on pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on PR #29822:
URL: https://github.com/apache/airflow/pull/29822#issuecomment-1452102745
Nope: This actually changing into the documentation, now some methods have their own pages, see: https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/sts.html
--
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] o-nikolas merged pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas merged PR #29822:
URL: https://github.com/apache/airflow/pull/29822
--
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] ferruzzi commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1122097976
##########
tests/providers/amazon/aws/utils/test_waiter.py:
##########
@@ -38,6 +38,17 @@ def generate_response(state: str) -> dict[str, Any]:
}
+def assert_expected_waiter_type(waiter: mock.MagicMock, expected: str):
+ """
+ There does not appear to be a straight-forward way to assert the type of waiter.
+ Instead, get the class name and check if it contains the expected name.
+
+ :param waiter: A mocked Boto3 Waiter object.
+ :param expected: The expected class name of the Waiter object, for example "ClusterActive".
+ """
+ assert expected in str(type(waiter.call_args[0][0]))
Review Comment:
Not a bad idea. I wonder if that test should live in the service hook unit tests instead of in test_custom_waiters.py? So each service hook which presents custom waiters would assert that it is, in fact, presenting those waiters.
--
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] Taragolis commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1122106803
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -191,8 +196,23 @@ def __init__(
aws_conn_id: str = "aws_default",
waiter_countdown: int = 25 * 60,
waiter_check_interval_seconds: int = 60,
+ # TODO: waiter_max_attempts and waiter_delay should default to None when the other two are deprecated.
+ waiter_max_attempts: int | ArgNotSet = NOTSET,
+ waiter_delay: int | ArgNotSet = NOTSET,
**kwargs: Any,
):
+ if waiter_max_attempts is NOTSET:
+ warnings.warn(
+ "The parameter waiter_countdown has been deprecated to standardize "
+ "naming conventions. Please use waiter_max_attempts instead. In the "
+ "future this will default to None and defer to the waiter's default value."
+ )
Review Comment:
It is not about ignore deprecated one, just suggestion for calculate `waiter_max_attempts` on own condition so we do not need to check `waiter_max_attempts is NOTSET` twice.
But anyway it is just minor suggestion not affect to anything critical, we could keep it as it is.
--
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] ferruzzi commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1122247015
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -191,8 +196,23 @@ def __init__(
aws_conn_id: str = "aws_default",
waiter_countdown: int = 25 * 60,
waiter_check_interval_seconds: int = 60,
+ # TODO: waiter_max_attempts and waiter_delay should default to None when the other two are deprecated.
+ waiter_max_attempts: int | ArgNotSet = NOTSET,
+ waiter_delay: int | ArgNotSet = NOTSET,
**kwargs: Any,
):
+ if waiter_max_attempts is NOTSET:
+ warnings.warn(
+ "The parameter waiter_countdown has been deprecated to standardize "
+ "naming conventions. Please use waiter_max_attempts instead. In the "
+ "future this will default to None and defer to the waiter's default value."
+ )
Review Comment:
I totally missed that you were assigning the value inside the deprecation block :facepalm: Yeah, that makes perfect sense and I'll make the change.... after I grab another coffee, apparently... Sorry about 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ferruzzi commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1122320795
##########
tests/providers/amazon/aws/utils/test_waiter.py:
##########
@@ -38,6 +38,17 @@ def generate_response(state: str) -> dict[str, Any]:
}
+def assert_expected_waiter_type(waiter: mock.MagicMock, expected: str):
+ """
+ There does not appear to be a straight-forward way to assert the type of waiter.
+ Instead, get the class name and check if it contains the expected name.
+
+ :param waiter: A mocked Boto3 Waiter object.
+ :param expected: The expected class name of the Waiter object, for example "ClusterActive".
+ """
+ assert expected in str(type(waiter.call_args[0][0]))
Review Comment:
I'll add this check in the EMR hook unit tests, I think it's a good suggestion.
--
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] ferruzzi commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1120809487
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -204,8 +224,12 @@ def __init__(
self.wait_for_completion = wait_for_completion
self.cluster_id = cluster_id
self.aws_conn_id = aws_conn_id
- self.waiter_countdown = waiter_countdown
- self.waiter_check_interval_seconds = waiter_check_interval_seconds
+ self.waiter_max_attempts = (
+ (waiter_countdown // waiter_check_interval_seconds)
+ if waiter_max_attempts is NOTSET
+ else waiter_max_attempts
+ )
+ self.waiter_delay = waiter_check_interval_seconds if waiter_delay is NOTSET else NOTSET
Review Comment:
This logic could be way cleaner if NOTSET returned a falsy value. Slack thread to discuss that change [here](https://apache-airflow.slack.com/archives/CCPRP7943/p1677618632397219). Currently NOTSET does not have a `__bool__` method so `self.waiter_delay = waiter_delay or waiter_check_interval_seconds` would set `self.waiter_delay` to NOTSET.
--
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] ferruzzi commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1122320795
##########
tests/providers/amazon/aws/utils/test_waiter.py:
##########
@@ -38,6 +38,17 @@ def generate_response(state: str) -> dict[str, Any]:
}
+def assert_expected_waiter_type(waiter: mock.MagicMock, expected: str):
+ """
+ There does not appear to be a straight-forward way to assert the type of waiter.
+ Instead, get the class name and check if it contains the expected name.
+
+ :param waiter: A mocked Boto3 Waiter object.
+ :param expected: The expected class name of the Waiter object, for example "ClusterActive".
+ """
+ assert expected in str(type(waiter.call_args[0][0]))
Review Comment:
I'll add this check in the EMR hook unit tests, I think it's a good suggestion. May not get to it till tomorrow though.
--
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] ferruzzi commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1122092558
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -191,8 +196,23 @@ def __init__(
aws_conn_id: str = "aws_default",
waiter_countdown: int = 25 * 60,
waiter_check_interval_seconds: int = 60,
+ # TODO: waiter_max_attempts and waiter_delay should default to None when the other two are deprecated.
+ waiter_max_attempts: int | ArgNotSet = NOTSET,
+ waiter_delay: int | ArgNotSet = NOTSET,
**kwargs: Any,
):
+ if waiter_max_attempts is NOTSET:
+ warnings.warn(
+ "The parameter waiter_countdown has been deprecated to standardize "
+ "naming conventions. Please use waiter_max_attempts instead. In the "
+ "future this will default to None and defer to the waiter's default value."
+ )
Review Comment:
I'm not sure I follow. `waiter_countdown` and `waiter_check_interval_seconds` both already exist and we are deprecating those, but we can't just ignore them or that would break everyone currently using them.
--
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] o-nikolas commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1121037262
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -204,8 +224,12 @@ def __init__(
self.wait_for_completion = wait_for_completion
self.cluster_id = cluster_id
self.aws_conn_id = aws_conn_id
- self.waiter_countdown = waiter_countdown
- self.waiter_check_interval_seconds = waiter_check_interval_seconds
+ self.waiter_max_attempts = (
+ (waiter_countdown // waiter_check_interval_seconds)
+ if waiter_max_attempts is NOTSET
+ else waiter_max_attempts
+ )
+ self.waiter_delay = waiter_check_interval_seconds if waiter_delay is NOTSET else NOTSET
Review Comment:
Maybe I'm confused, but don't you want this?:
```suggestion
self.waiter_delay = waiter_check_interval_seconds if waiter_delay is NOTSET else waiter_delay
```
--
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] ferruzzi commented on pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #29822:
URL: https://github.com/apache/airflow/pull/29822#issuecomment-1451014690
Wow. Something blew up the AWS docs. :eyes:
--
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] Taragolis commented on pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on PR #29822:
URL: https://github.com/apache/airflow/pull/29822#issuecomment-1452066767
> Wow. Something blew up the AWS docs. 👀
>
> [EDIT] Looks like a bunch are failing for doc builds, I'll see if I can sort it out
This error refers to something changed in inter sphinx inventory which provided by `boto3`, let's run it again maybe in transitional error
--
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] Taragolis commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1122101239
##########
tests/providers/amazon/aws/utils/test_waiter.py:
##########
@@ -38,6 +38,17 @@ def generate_response(state: str) -> dict[str, Any]:
}
+def assert_expected_waiter_type(waiter: mock.MagicMock, expected: str):
+ """
+ There does not appear to be a straight-forward way to assert the type of waiter.
+ Instead, get the class name and check if it contains the expected name.
+
+ :param waiter: A mocked Boto3 Waiter object.
+ :param expected: The expected class name of the Waiter object, for example "ClusterActive".
+ """
+ assert expected in str(type(waiter.call_args[0][0]))
Review Comment:
Good question. I think as more hooks will provide custom waiter as bigger would be `test_custom_waiters.py` so I thought move waiters tests to specific hook would be reasonable
--
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] Taragolis commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1121525813
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -191,8 +196,23 @@ def __init__(
aws_conn_id: str = "aws_default",
waiter_countdown: int = 25 * 60,
waiter_check_interval_seconds: int = 60,
+ # TODO: waiter_max_attempts and waiter_delay should default to None when the other two are deprecated.
+ waiter_max_attempts: int | ArgNotSet = NOTSET,
+ waiter_delay: int | ArgNotSet = NOTSET,
**kwargs: Any,
):
+ if waiter_max_attempts is NOTSET:
+ warnings.warn(
+ "The parameter waiter_countdown has been deprecated to standardize "
+ "naming conventions. Please use waiter_max_attempts instead. In the "
+ "future this will default to None and defer to the waiter's default value."
+ )
Review Comment:
```suggestion
if waiter_max_attempts is NOTSET:
warnings.warn(
"The parameter waiter_countdown has been deprecated to standardize "
"naming conventions. Please use waiter_max_attempts instead. In the "
"future this will default to None and defer to the waiter's default value."
)
waiter_max_attempts = waiter_countdown // waiter_check_interval_seconds
```
What if we change value here so we could change this
```python
self.waiter_max_attempts = (
(waiter_countdown // waiter_check_interval_seconds)
if waiter_max_attempts is NOTSET
else waiter_max_attempts
)
```
to
```python
self.waiter_max_attempts = waiter_max_attempts
```
##########
tests/providers/amazon/aws/utils/test_waiter.py:
##########
@@ -38,6 +38,17 @@ def generate_response(state: str) -> dict[str, Any]:
}
+def assert_expected_waiter_type(waiter: mock.MagicMock, expected: str):
+ """
+ There does not appear to be a straight-forward way to assert the type of waiter.
+ Instead, get the class name and check if it contains the expected name.
+
+ :param waiter: A mocked Boto3 Waiter object.
+ :param expected: The expected class name of the Waiter object, for example "ClusterActive".
+ """
+ assert expected in str(type(waiter.call_args[0][0]))
Review Comment:
Cross-PR suggestion
I've checked that specific hook has access to waiters, in my case only custom waiters
https://github.com/apache/airflow/blob/cc8ce585cad5849a561404cf3d2cb726c207a2cb/tests/providers/amazon/aws/waiters/test_custom_waiters.py#L127-L132
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -159,9 +160,11 @@ class EmrStartNotebookExecutionOperator(BaseOperator):
group to associate with the master instance of the EMR cluster for this notebook execution.
:param tags: Optional list of key value pair to associate with the notebook execution.
:param waiter_countdown: Total amount of time the operator will wait for the notebook to stop.
- Defaults to 25 * 60 seconds.
+ Defaults to 25 * 60 seconds. (Deprecated. Please use waiter_max_attempts.)
:param waiter_check_interval_seconds: Number of seconds between polling the state of the notebook.
- Defaults to 60 seconds.
Review Comment:
Not strong opinion, but what if we move deprecated values to the bottom of docstring and `__init__` signature.
It should not affect anything because Operators only use keyword arguments however less important arguments would be in the bottom
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -191,8 +196,23 @@ def __init__(
aws_conn_id: str = "aws_default",
waiter_countdown: int = 25 * 60,
waiter_check_interval_seconds: int = 60,
+ # TODO: waiter_max_attempts and waiter_delay should default to None when the other two are deprecated.
+ waiter_max_attempts: int | ArgNotSet = NOTSET,
+ waiter_delay: int | ArgNotSet = NOTSET,
Review Comment:
```suggestion
waiter_max_attempts: int | None |ArgNotSet = NOTSET,
waiter_delay: int | None | ArgNotSet = NOTSET,
```
I guess None it is already valid value for this arguments
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -191,8 +196,23 @@ def __init__(
aws_conn_id: str = "aws_default",
waiter_countdown: int = 25 * 60,
waiter_check_interval_seconds: int = 60,
+ # TODO: waiter_max_attempts and waiter_delay should default to None when the other two are deprecated.
+ waiter_max_attempts: int | ArgNotSet = NOTSET,
+ waiter_delay: int | ArgNotSet = NOTSET,
**kwargs: Any,
):
+ if waiter_max_attempts is NOTSET:
+ warnings.warn(
+ "The parameter waiter_countdown has been deprecated to standardize "
+ "naming conventions. Please use waiter_max_attempts instead. In the "
+ "future this will default to None and defer to the waiter's default value."
+ )
Review Comment:
And the same to waiter_delay
--
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] ferruzzi commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1122093323
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -159,9 +160,11 @@ class EmrStartNotebookExecutionOperator(BaseOperator):
group to associate with the master instance of the EMR cluster for this notebook execution.
:param tags: Optional list of key value pair to associate with the notebook execution.
:param waiter_countdown: Total amount of time the operator will wait for the notebook to stop.
- Defaults to 25 * 60 seconds.
+ Defaults to 25 * 60 seconds. (Deprecated. Please use waiter_max_attempts.)
:param waiter_check_interval_seconds: Number of seconds between polling the state of the notebook.
- Defaults to 60 seconds.
Review Comment:
Sure, I suppose there's no harm in that. I can make the change.
--
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] ferruzzi commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1122094891
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -191,8 +196,23 @@ def __init__(
aws_conn_id: str = "aws_default",
waiter_countdown: int = 25 * 60,
waiter_check_interval_seconds: int = 60,
+ # TODO: waiter_max_attempts and waiter_delay should default to None when the other two are deprecated.
+ waiter_max_attempts: int | ArgNotSet = NOTSET,
+ waiter_delay: int | ArgNotSet = NOTSET,
Review Comment:
Ah, yep, good catch. I'll make the change.
--
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] o-nikolas commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1122234260
##########
tests/providers/amazon/aws/utils/test_waiter.py:
##########
@@ -38,6 +38,17 @@ def generate_response(state: str) -> dict[str, Any]:
}
+def assert_expected_waiter_type(waiter: mock.MagicMock, expected: str):
+ """
+ There does not appear to be a straight-forward way to assert the type of waiter.
+ Instead, get the class name and check if it contains the expected name.
+
+ :param waiter: A mocked Boto3 Waiter object.
+ :param expected: The expected class name of the Waiter object, for example "ClusterActive".
+ """
+ assert expected in str(type(waiter.call_args[0][0]))
Review Comment:
> I wonder if that test should live in the service hook unit tests instead of in test_custom_waiters.py?
+1
--
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] ferruzzi commented on a diff in pull request #29822: Implement custom boto waiters for some EMR operators
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29822:
URL: https://github.com/apache/airflow/pull/29822#discussion_r1121089376
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -204,8 +224,12 @@ def __init__(
self.wait_for_completion = wait_for_completion
self.cluster_id = cluster_id
self.aws_conn_id = aws_conn_id
- self.waiter_countdown = waiter_countdown
- self.waiter_check_interval_seconds = waiter_check_interval_seconds
+ self.waiter_max_attempts = (
+ (waiter_countdown // waiter_check_interval_seconds)
+ if waiter_max_attempts is NOTSET
+ else waiter_max_attempts
+ )
+ self.waiter_delay = waiter_check_interval_seconds if waiter_delay is NOTSET else NOTSET
Review Comment:
100% my mistake. Thanks for the catch.
--
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