o-nikolas commented on code in PR #30032:
URL: https://github.com/apache/airflow/pull/30032#discussion_r1134674392


##########
airflow/providers/amazon/aws/hooks/base_aws.py:
##########
@@ -42,6 +42,7 @@
 import botocore.session
 import requests
 import tenacity
+from aiobotocore.session import AioSession, get_session as async_get_session

Review Comment:
   >    `aiobotocore` pin `botocore` and `boto3` for specific version, last 
version update takes 7 month, if compare with regular boto stuff it have 3-5 
updates per week: fixes, new AWS services and etc. So my opinion that this 
should not be a part of core dependencies, only if we want to make one or two 
steps back.
   > 
   That 7 month window is kind of the worst case. I'm seeing aiobotocore 
releases which bump botocore as short as one month (and everything in between). 
   >There was plan to add more consistency (not early than 30 Apr) in hooks and 
add type hinting by separate existed hooks by [Amazon Provider: Consistency in 
boto3-based Hooks #28560](https://github.com/apache/airflow/discussions/28560) 
by separate base hooks. And `aiobotocore.session.AioSession` is not a 
replacement of `boto3.session.Session`.
   
   Agree with @eladkal on this one.
   >`aiobotocore` can't work with `moto` and some critical stuff tested by 
`moto`, e.g. auth and AssumeRole
   
   I personally haven't looked into this. But, sure, we may have to do more 
manual mocking and testing for the aio usecase.
    
   >My opinion still the same, Airflow core not ready for async stuff 
`¯\_(ツ)_/¯`, all DB queries are use io blocking implementation (the regular 
one), so obtain credentials from config still required use some hacks, like 
`asgiref.sync.sync_to_async` which is transform triggerer service to [some kind 
of sequential 
executor](https://github.com/apache/airflow/pull/29038#discussion_r1100693885)
   
   Honestly, I think this ship has long since sailed. I agree that the 
architecture of deferrable operators/trigger is a bit clunky, leads to a lot of 
code duplication, and has this duality of sync vs async hooks. But the fact of 
the matter is Deferrable Operator support is public and we hear from users and 
customers that they want to use it. So I think an incremental/agile approach is 
often the best way to approach it. If we wait until there is perfection, we'd 
never ship anything.
   
   Maybe a helpful exercise would be to list some concrete examples of how this 
PR would degrade an Airflow user's experience more than it would improve it 
(other than the botocore version, that one is quite clear).  
   



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

Reply via email to