dstandish commented on PR #27947: URL: https://github.com/apache/airflow/pull/27947#issuecomment-1338756936
one thing to keep in mind.... we more or less have a convention, especially with aws since we have the helpful boto3 stubs library, that we should not add "alias methods" to hooks... i.e. when the underlying client method works perfectly well by itself, we should not add a method to the hook that simply forwards the kwargs to boto3. that just adds maintenance and a layer that users may need to decypher these aws hooks are designed in such a way that autocomplete works for boto3 client methods e.g. see here <img width="619" alt="image" src="https://user-images.githubusercontent.com/15932138/205817192-4ef125a7-26ed-4ccb-811b-49a26004579a.png";> which means, in many cases, no need to add the hook method -- just use the client. and that's what this operator does and that's a good thing. more code sharing isn't always better because it means it's tougher to make changes. if we add meaningful functionality to the hook then sure, add a method. but i think in this case `execute_query` doesn't quite cross that threshold. i can see wait_for_result potentially being useful but, if you would not mind, i would recommend simply going straight to your transfer operator and then doing any _necessary_ refactors as part of that, rather than saying "this is needed for this other pr" because... verifying that is easier to do when we have the actual PR. -- 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]
![https://user-images.githubusercontent.com/15932138/205817192-4ef125a7-26ed-4ccb-811b-49a26004579a.png"](https://user-images.githubusercontent.com/15932138/205817192-4ef125a7-26ed-4ccb-811b-49a26004579a.png"){kind=link}