Skip to content

Conversation

@cruseakshay
Copy link
Contributor

Closes: #55724

Fix unclosed aiohttp.ClientSession and TCPConnector warnings when using AzureDataFactoryRunPipelineOperator with deferrable=True.

The AzureDataFactoryAsyncHook creates an AsyncDataFactoryManagementClient but never closes it, causing resource leak errors in logs regardless of pipeline success/failure.

Changes

  • Add __aenter__, __aexit__, and close() methods to AzureDataFactoryAsyncHook
  • Update triggers to use async with hook: for proper cleanup
  • Add unit tests for context manager behavior

Follows the same pattern as #52119 (Databricks fix).


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: Claude following the guidelines


Copy link
Contributor

@SameerMesiah97 SameerMesiah97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Turning the async hook into a context manager and updating the trigger call sites fixes the aiohttp client leak cleanly. Tests look solid as well.

Not a blocker but I would suggest double checking if there any places outside this file where this hook is being called so that the leak does not persist after your fix.

@cruseakshay
Copy link
Contributor Author

@SameerMesiah97 Double checked other places, looks good.

@cruseakshay cruseakshay requested a review from dabla January 20, 2026 04:05
Copy link
Contributor

@dabla dabla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 small nits but looking good!

Copy link
Contributor

@dabla dabla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good to me.

@dabla
Copy link
Contributor

dabla commented Jan 22, 2026

@cruseakshay Did you actually tests this changes and can you confirm this works?

@dabla
Copy link
Contributor

dabla commented Jan 22, 2026

@cruseakshay Also, the changes made in the data_factory trigger, like the added _build_trigger_event method like I asked, would be nice to see a test for it as well. Is the trigger data_factory tests also testing the context manger?

@cruseakshay
Copy link
Contributor Author

cruseakshay commented Jan 23, 2026

@cruseakshay Did you actually tests this changes and can you confirm this works?

Yes, here are trail logs from latest run

[2026-01-23T04:53:13.235944Z] INFO - Sleeping for 60. The pipeline state is Queued.
[2026-01-23T04:54:13.245821Z] INFO - Request URL: 'https://management.azure.com/subscriptions/REDACTED/resourceGroups/airflow-adf-test-rg/providers/Microsoft.DataFactory/factories/airflow-adf-test/pipelineruns/REDACTED?api-version=REDACTED'
Request method: 'GET'
Request headers:
    'Accept': 'application/json'
    'x-ms-client-request-id': '9001a594-f817-11f0-b40d-b64b850643a2'
    'User-Agent': 'azsdk-python-mgmt-datafactory/9.2.0 Python/3.12.12 (Linux-6.12.54-linuxkit-aarch64-with-glibc2.36)'
    'Authorization': 'REDACTED'
No body was attached to the request
[2026-01-23T04:54:13.973572Z] INFO - Response status: 200
Response headers:
    'Cache-Control': 'no-cache'
    'Pragma': 'no-cache'
    'Content-Length': '816'
    'Content-Type': 'application/json; charset=utf-8'
    'Expires': '-1'
    'x-ms-correlation-request-id': 'REDACTED'
    'x-ms-operation-identifier': 'REDACTED'
    'x-ms-ratelimit-remaining-subscription-reads': '249'
    'x-ms-ratelimit-remaining-subscription-global-reads': 'REDACTED'
    'x-ms-request-id': '59f3995c-910c-4abe-952a-5e3e8fa3bff3'
    'x-ms-routing-request-id': 'REDACTED'
    'Strict-Transport-Security': 'REDACTED'
    'X-Content-Type-Options': 'REDACTED'
    'X-Cache': 'REDACTED'
    'X-MSEdge-Ref': 'Ref A: 1C93C26BC61B478A8D2F48098F9A042C Ref B: MAA201060513029 Ref C: 2026-01-23T04:54:13Z'
    'Date': 'Fri, 23 Jan 2026 04:54:13 GMT'
[2026-01-23T04:54:13.974417Z] INFO - Trigger fired eventname=test_adf_trigger_fix/manual__2026-01-23T04:53:02.479036+00:00/run_pipeline_deferrable/-1/1 (ID 1) result=TriggerEvent<{'status': 'success', 'message': 'The pipeline run 66bb30ba-f817-11f0-bec5-16db0266592f has Succeeded.', 'run_id': '66bb30ba-f817-11f0-bec5-16db0266592f'}> 
[2026-01-23T04:54:13.975233Z] INFO - trigger completedname=test_adf_trigger_fix/manual__2026-01-23T04:53:02.479036+00:00/run_pipeline_deferrable/-1/1 (ID 1) 
[2026-01-23T04:54:15.380187Z] INFO - DAG bundles loaded: dags-folder
[2026-01-23T04:54:15.380487Z] INFO - Filling up the DagBag from /usr/local/airflow/dags/test_adf_trigger_fix.py
[2026-01-23T04:54:15.488568Z] INFO - TaskInstance Details: dag_id=test_adf_trigger_fix task_id=run_pipeline_deferrable dagrun_id=manual__2026-01-23T04:53:02.479036+00:00 map_index=-1 run_start_date=2026-01-23T04:54:15.366708Z try_number=1 op_classpath=["airflow.providers.microsoft.azure.operators.data_factory.AzureDataFactoryRunPipelineOperator"] 
[2026-01-23T04:54:15.488765Z] INFO - The pipeline run 66bb30ba-f817-11f0-bec5-16db0266592f has Succeeded.
[2026-01-23T04:54:15.508872Z] WARNING - Connection schemes (type: azure_data_factory) shall not contain '_' according to RFC3986.

@cruseakshay
Copy link
Contributor Author

cruseakshay commented Jan 23, 2026

@cruseakshay Also, the changes made in the data_factory trigger, like the added _build_trigger_event method like I asked, would be nice to see a test for it as well. Is the trigger data_factory tests also testing the context manger?

hook-level test that covers the context manager behavior: AzureDataFactoryAsyncHook has test_context_manager_calls_close that explicitly verifies async with hook: calls close(). Haven't seen any trigger tests explicitly assert __aenter__/__aexit__ calls.

@dabla
Copy link
Contributor

dabla commented Jan 27, 2026

@cruseakshay Thanks for the PR and your patience, good job!

@dabla dabla merged commit 033d185 into apache:main Jan 27, 2026
89 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Jan 27, 2026

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

shreyas-dev pushed a commit to shreyas-dev/airflow that referenced this pull request Jan 29, 2026
…he#60650)

* Fix unclosed aiohttp ClientSession in AzureDataFactoryAsyncHook

* If cancel_pipeline_run() creates a new connection

* Address review comments

* review: early return

* add tests: build_trigger_event

---------

Co-authored-by: Akshay <cruseakshay@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unclosed aiohttp ClientSession and TCPConnector after AzureDataFactoryRunPipelineOperator execution

3 participants