-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Handle BaseHook.get_connection's new Task SDK exception when connection is not found
#47648
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
Handle BaseHook.get_connection's new Task SDK exception when connection is not found
#47648
Conversation
BaseHook.get_connection's new Task SDK exception in AwsGenericHookBaseHook.get_connection's new Task SDK exception
BaseHook.get_connection's new Task SDK exceptionBaseHook.get_connection's new Task SDK exception when connection is not found
|
@ramitkataria is this a duplicate of #47593? |
I don't think so? It was still raising |
Yep, i noticed it. Thanks for your contribution @ramitkataria |
|
@amoghrajesh Is all your feedback addressed? Does this look ready to approve? |
|
Needs rebase and resolve conflicts |
ac7890c to
a1bb15e
Compare
…en appropriate When a Task SDK connection is not found, it currently throws AirflowRuntimeError. This catches it in this case and raises AirflowNotFoundException for consistency.
a1bb15e to
ee7d4d2
Compare
…tion is not found (apache#47648) When a Task SDK connection is not found, it currently throws AirflowRuntimeError. This PR catches it in this case and raises AirflowNotFoundException for consistency.
…tion is not found (apache#47648) When a Task SDK connection is not found, it currently throws AirflowRuntimeError. This PR catches it in this case and raises AirflowNotFoundException for consistency.
…tion is not found (apache#47648) When a Task SDK connection is not found, it currently throws AirflowRuntimeError. This PR catches it in this case and raises AirflowNotFoundException for consistency.
…tion is not found (apache#47648) When a Task SDK connection is not found, it currently throws AirflowRuntimeError. This PR catches it in this case and raises AirflowNotFoundException for consistency.
In one of my previus PRs (apache#47648), I incorrectly assumed that catching the exception from `secrets_backend.get_connection` was meant for the connection not found case. So, I'm reversing a couple of changes from that PR for consistency.
* Fix consistency in exception/log handling in `connection.py` In one of my previus PRs (#47648), I incorrectly assumed that catching the exception from `secrets_backend.get_connection` was meant for the connection not found case. So, I'm reversing a couple of changes from that PR for consistency. * Change log message
…50329) * Fix consistency in exception/log handling in `connection.py` In one of my previus PRs (apache#47648), I incorrectly assumed that catching the exception from `secrets_backend.get_connection` was meant for the connection not found case. So, I'm reversing a couple of changes from that PR for consistency. * Change log message
When a Task SDK connection is not found, it currently throws
AirflowRuntimeError. This PR catches it in this case and raisesAirflowNotFoundExceptionfor consistency.Earlier, I was changing
AwsGenericHookto also catch theAirflowRuntimeErrorbut I was getting import errors in tests. Then I realized it's probably better if this error handling is done where it was being done previously.After #47048, the boto fallback strategy started breaking inAwsGenericHook(explained in more detail here:#47048 (comment)). I realized that this was because a new exception
AirflowRuntimeErrorwas introduced inBaseHook'sget_connection. It used to only throwAirflowNotFoundExceptionearlier.^ 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.