potiuk commented on code in PR #30032:
URL: https://github.com/apache/airflow/pull/30032#discussion_r1136237987


##########
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:
   > Till native and conflictless approach introduced, maybe async operators 
should be external in their own dedicated package and to be installed by users 
who seek to use triggers? 
   
   That is also a possibility of course. The end result is though, that if in 
the "sync" package you would like to use some new feature you technically 
should use ">=version_when_it_was_added".
   
   Byt you really can't add such limit - If you add such limit in the "sync" 
package, then you won't be able to install those two packages together (which I 
guess is out-of-the-question). Effectively this means that we will cause 
breaking some "sync" package features by installing the "async" package. Not 
even mentioning installing "older" version of the "async" package with newer 
version of "sync" one - this will break even more things and quite 
unpredictably. I already imagine the issues raised by the users :).
   
   It will also cause quite a lot of duplication of the code which necessarily 
will have to be in both packages. I think sharing a code between two packages 
like that will be quite complex. IMHO it's far more complex to maintain, fix 
problems, make sure that there is some level of compatibility and handling 
errors will be a nightmare to maintain. 
   
   I on the other hand, I am worried we may underestimate complexities of 
maintaining such split :) . Effectively we take the burden on the maintainers/ 
aws team to handle all the complexity involved rather than leave it up to the 
users to slightly complicate their workflows if they want to be on the 
"bleeding edge". 
   
   But if AWS team is ok to maintain such a split -  I am ok with this scenario 
as well - it is certainly doable. Adding new `asyncaws` provider could be done. 
Maybe even extracting `common.aws` if we would like to share the code and those 
who do it will not be afraid of handling cases like we had with common.sql - I 
just believe it will be super-complex to maintain. You've been warned :). 



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