Uh oh!
There was an error while loading. Please reload this page.
Handle task-sdk connection retrieval import error in BaseHook - #54692
Conversation
potiuk
left a comment
There was a problem hiding this comment.
I think that will be fixed in final 3.1 (@amoghrajesh ?) - but for now it's good as it is.
Uh oh!
There was an error while loading. Please reload this page.
potiuk
commented
Aug 20, 2025
Following the comment of @amoghrajesh -> this actualy should be fixed by installing Question @seanghaeli -> is there something "special" you do to get the environment for system tests? |
o-nikolas
commented
Aug 20, 2025
I'm curious about this, we're certainly using the task API and we're not doing anything special beyond running the breeze system tests commands.
This is interesting, the image we use for most test workers is either breeze itself (in the local case) or the apache airflow public image from dockerhub. If task-sdk isn't installed in either we can be sure to do that, but shouldn't it be there already @potiuk? |
amoghrajesh
commented
Aug 20, 2025
@o-nikolas yes it should. Could either of you show me some of the failures so that we can handle that better if needed? But long term (3.1+), task SDK should be the way to do these things. |
potiuk
commented
Aug 20, 2025
Task SDK is not installed in Airflow 2 - maybe you are using those when running system tests on it? |
o-nikolas
commented
Aug 20, 2025
We're running on main (so effectively 3.1) this is how this recent change affected our builds |
seanghaeli
commented
Aug 20, 2025
@amoghrajesh the end of the stacktrace I was seeing is: This PR provides the workaround so that the testing infrastructure can handle the thrown error and then fall back to default credentials. |
o-nikolas
commented
Aug 22, 2025
@amoghrajesh Re:
Do you need anything further from @seanghaeli ? |
o-nikolas
commented
Sep 3, 2025
Hey @amoghrajesh We're seeing this in our Kubernetes system tests as well, when run in deferrable mode specifically. Here is a traceback: I would prefer to not put another catch-log-ignore in the cncf hook like we did here for the amazon provider (I'd preferably like to remove that one also). Is it possible for you to have a look at this one? |
I've done a lot of deep diving and I think I've identified what's happening here. So the above exception is actually coming from the reentry point when we're returning from the triggerer. Just before that exception in the logs we see: And I see successful communication with task API comms above that. So it's actually within the Triggerer scope that we're failing to communicate with task api to get connection information. This is because in the dag.test() world, we run triggerers inline here and we do not setup the inprocess supervisor comms like we do for regular task execution in dag.test() here. So I believe we need to update the inline execution of triggers in dag.test() to setup a supervisor comms. I have a draft PR here with what I think that might look like. But likely needs refinement: #55236 |
When these lines were merged from #54083:
https://github.com/apache/airflow/pull/54083/files/5f2f2d8c95dfc4bf9bc98c7f9c3bb97f34062da6#diff-acfa34467241916c1189174e98f87a4094e8cbf58d391fa084e6afb2594decbcL61-L71
The Amazon provider system tests all started failing because the testing infrastructure does not use task-sdk. An ImportError is thrown when the testing infrastructure tries to retrieve the connection via task-sdk. In this PR, we handle this import error and the testing infrastructure then falls back to the default connection, which is the standard way the tests are run. With this change, the tests work.