vandonr-amz commented on code in PR #30032:
URL: https://github.com/apache/airflow/pull/30032#discussion_r1152442059
##########
airflow/providers/amazon/aws/hooks/base_aws.py:
##########
@@ -634,6 +661,14 @@ def conn(self) -> BaseAwsConnection:
else:
return self.get_resource_type(region_name=self.region_name)
+ @cached_property
Review Comment:
I don't think this should be a cached property. It returns a
ClientCreatorContext, which is meant to be used in a `with` block. And
aiobotocore is written so that the coroutine creating the client is awaited
when entering the block:
https://github.com/aio-libs/aiobotocore/blob/72b8dd5d7d4ef2f1a49a0ae0c37b47e5280e2070/aiobotocore/session.py#L26-L27
So if we cache the context, trying to using twice (i.e. call `__enter__`
twice) will result in awaiting the same coroutine twice, which throws an
exception.
What we can do is either:
* cache the AioBaseClient by calling manually __aenter__ (but it feels
wrong)
* do not cache (but then we recreate the client each time, might be a perf
issue or not)
* find something else to cache mid-way, like the AioSession (but it's more
complex to setup here, and depending on where the time is spent, might not help
much)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
