Avoid extra_dejson in ADF and Synapse async hooks - #72130
Open
Vamsi-klu wants to merge 1 commit into
Open
Conversation
extra_dejson can mask secrets via a sync send on the triggerer event loop, which raises AsyncToSync. Parse extras with json.loads instead, matching the MSGraph workaround. closes: apache#55728
Contributor
Author
|
Could you review the leftover extra_dejson and sync get_connection hops on the ADF and Synapse async hooks? This is the same AsyncToSync triggerer-loop failure as #55179, now in AzureDataFactoryAsyncHook.get_async_conn / provide_targeted_factory_async and AzureSynapsePipelineAsyncHook.get_async_conn. The question is whether json.loads(conn.extra) on the async path, and get_async_connection in the Synapse hook, is the right leftover fix, and whether leaving the sync hooks on extra_dejson is correct. Drafted-by: Cursor Grok 4.6 (no human review before posting) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes: #55728
What is the change?
The async paths in the Azure Data Factory and Synapse pipeline hooks stop reading
conn.extra_dejsonand parse extras withjson.loads(conn.extra)instead.AzureSynapsePipelineAsyncHook.get_async_connnow fetches the connection throughget_async_connectionrather than the syncget_connection, and ADFrefresh_conncloses the cached_async_connbefore rebuilding it.Why did I do it?
extra_dejsoncan callmask_secret, which does a sync send on the triggerer event loop and raisesRuntimeError: You cannot use AsyncToSync in the same thread as an async event loop. The stack users see atexecute_completeis that trigger error re-raised. This is the same leftover that #55179 fixed for MSGraph.How did I do it?
In
hooks/data_factory.pyandhooks/synapse.pyI swappedconn.extra_dejsonforjson.loads(conn.extra) if conn.extra else {}on the async paths only, and maderefresh_connawaitself.close(). The sync hooks keepextra_dejsonso worker-side secret masking is unchanged.What's the impact?
Deferrable ADF and Synapse pipeline runs stop crashing on the triggerer with the AsyncToSync error. Sync hooks and
execute_completebehave exactly as before. Azure provider only.What's the test plan?
New unit tests make
extra_dejsonraise the real AsyncToSync error and assert the async hooks still build their clients; they fail ifextra_dejsoncomes back.test_refresh_connnow asserts the aio client gets closed. Reviewers can re-run the two touched test files orbreeze testing providers-tests --test-type "Providers[microsoft.azure]"; provider CI covers the same.Was generative AI tooling used to co-author this PR?
Generated-by: Cursor Grok 4.6 following the guidelines