-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Add a backup implementation in AWS MwaaHook for calling the MWAA API #47035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
After discussion with @o-nikolas, I ended up removing the storage of sessions in hook instances because they would get re-instantiated again anyway. The only feasible way seems to be to serialize, encrypt and store them in the file system, which can be an improvement implemented in the future. I was going to implement retries using tenacity so that the hook would wait instead of failing in case of throttling but I did some performance testing to see the how it would handle a throttling scenario and it looks like boto waits instead of returning an error response so that shouldn't happen. Either way, if someone does manage to get errors related to throttling and wants the hook to retry instead of fail, boto has this guide about setting up retries which seems like the best way to go about this. |
providers/amazon/src/airflow/providers/amazon/aws/hooks/mwaa.py
Outdated
Show resolved
Hide resolved
providers/amazon/src/airflow/providers/amazon/aws/hooks/mwaa.py
Outdated
Show resolved
Hide resolved
providers/amazon/src/airflow/providers/amazon/aws/hooks/mwaa.py
Outdated
Show resolved
Hide resolved
1b804cf to
3502f23
Compare
The existing implementation doesn't work when the user doesn't have `airflow:InvokeRestApi` permission in their IAM policy or when they make more than 10 transactions per second. This implementation mitigates those issues by using a session token approach. However, my existing implementation is still used by default because it is simpler. Some context here: https://docs.aws.amazon.com/mwaa/latest/userguide/access-mwaa-apache-airflow-rest-api.html
7a5d6d8 to
62a5eef
Compare
After [discussion in another PR](apache#47035 (comment)), I added this test to make sure we find out if the the IAM fallback in `MwaaHook` (meant to facilitate users who don't have `airflow:InvokeRestApi` policy in their AWS IAM role) ever stops working, possibly due to the error response changing.
After [discussion in another PR](apache#47035 (comment)), I added this test to make sure we find out if the the IAM fallback in `MwaaHook` (meant to facilitate users who don't have `airflow:InvokeRestApi` policy in their AWS IAM role) ever stops working, possibly due to the error response changing.
After [discussion in another PR](apache#47035 (comment)), I added this test to make sure we find out if the the IAM fallback in `MwaaHook` (meant to facilitate users who don't have `airflow:InvokeRestApi` policy in their AWS IAM role) ever stops working, possibly due to the error response changing.
…pache#47035) The existing implementation doesn't work when the user doesn't have `airflow:InvokeRestApi` permission in their IAM policy or when they make more than 10 transactions per second. This implementation mitigates those issues by using a session token approach. However, my existing implementation is still used by default because it is simpler. Some context here: https://docs.aws.amazon.com/mwaa/latest/userguide/access-mwaa-apache-airflow-rest-api.html
The existing implementation doesn't work when the user doesn't have
airflow:InvokeRestApipermission in their IAM policy or when they make more than 10 transactions per second.This implementation mitigates those issues by using a session token approach. However, my existing implementation is still used by default because it is simpler.
Some context here:
https://docs.aws.amazon.com/mwaa/latest/userguide/access-mwaa-apache-airflow-rest-api.html
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.