Skip to content

[Core] Fix #33996: az network vnet create: Eagerly import requests during CLI startup to avoid Python 3.14 module-lock deadlock - #33997

Open
Aditya Pujara (a0x1ab) wants to merge 2 commits into
Azure:devfrom
a0x1ab:agent-assist/azure-azure-cli-issue-33996-b92a89e7781c
Open

[Core] Fix #33996: az network vnet create: Eagerly import requests during CLI startup to avoid Python 3.14 module-lock deadlock#33997
Aditya Pujara (a0x1ab) wants to merge 2 commits into
Azure:devfrom
a0x1ab:agent-assist/azure-azure-cli-issue-33996-b92a89e7781c

Conversation

@a0x1ab

@a0x1ab Aditya Pujara (a0x1ab) commented Aug 27, 2026 •
edited by x-engineering-agent Bot
Loading

Copy link
Copy Markdown
Member

🤖 PR Validation — ️✔️ All clear

Breaking Changes Tests
️✔️ None ️✔️ 130/130

Description

Fixes #33996.

Related command
az network vnet create (and any command that constructs an MSAL credential)

Description

Python 3.14 detects import-lock ordering cycles and raises _DeadlockError instead of blocking forever. When msal lazily imports requests from a background/worker thread while the main thread is mid-import of a transitively-imported submodule (e.g. requests.structures), the per-module lock ordering creates a detectable cycle, causing intermittent failures like:

ERROR: deadlock detected by _ModuleLock('requests.structures')

Fix: eagerly import msal and import requests at module-load time in azure/cli/core/auth/identity.py. This guarantees both packages and all their submodules are fully initialised in sys.modules on the main thread before any credential-creation code runs on a worker thread, eliminating the race entirely.

Regression test added in tests/test_auth_eager_import.py: evicts requests, msal, and azure.cli.core.auth from sys.modules in setUp/tearDown, then re-imports identity and asserts requests, requests.structures, and msal are all present — ensuring the eager imports cannot be silently removed.

Testing Guide

cd src/azure-cli-core
python -m unittest azure.cli.core.tests.test_auth_eager_import -v

History Notes

[Core] az network vnet create: Fix intermittent Python 3.14 _DeadlockError on requests.structures by eagerly importing requests and msal at CLI startup

@a0x1ab
[Core] Fix Azure#33996: az network vnet create: Eagerly import requ…
d8abdad 
…ests during CLI startup to avoid Python 3.14 module-lock deadlock

* Initial plan

* [Core] Eagerly import requests/msal in auth/identity.py to avoid Python 3.14 deadlock

Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service Bot added the Auto-Assign Auto assign by bot label Aug 27, 2026
@x-engineering-agent
x-engineering-agent Bot marked this pull request as ready for review August 27, 2026 05:21
@x-engineering-agent
x-engineering-agent Bot requested a review from a team as a code owner August 27, 2026 05:21
Copilot AI lite review requested due to automatic review settings August 27, 2026 05:21
@x-engineering-agent
x-engineering-agent Bot requested review from a team as code owners August 27, 2026 05:21
@azure-pipelines

azure-pipelines Bot commented Aug 27, 2026

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI reviewed Aug 27, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses an intermittent Python 3.14 _DeadlockError seen in auth flows (e.g., az network vnet create) by ensuring relevant auth/networking dependencies are imported early enough to avoid import-lock ordering cycles under multi-threaded execution.

Changes:

  • Add eager imports in azure.cli.core.auth.identity intended to pre-load requests (and MSAL) before credential creation can trigger lazy imports on worker threads.
  • Add a regression unit test that evicts relevant modules from sys.modules, re-imports identity, and asserts expected modules are preloaded.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/azure-cli-core/azure/cli/core/auth/identity.py Adds eager imports to reduce risk of Python 3.14 module-lock deadlock during auth initialization.
src/azure-cli-core/azure/cli/core/tests/test_auth_eager_import.py Adds regression coverage asserting eager import behavior after a clean re-import.
Suppressed comments (1)

src/azure-cli-core/azure/cli/core/tests/test_auth_eager_import.py:70

  • This file appears to use pylint-style disables elsewhere in the repo; # noqa: F401 may not suppress the unused-import warning in CI. Prefer # pylint: disable=unused-import on this import statement.
        import azure.cli.core.auth.identity  # noqa: F401

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/azure-cli-core/azure/cli/core/auth/identity.py
Comment on lines +11 to 19
# Eagerly import requests and msal here, on the main thread, so that their
# submodules (e.g. requests.structures) are fully initialised in sys.modules
# before any background thread can trigger a lazy import. Python 3.14
# detects import-lock ordering cycles and raises _DeadlockError when two
# threads race to initialise the same module; pre-loading avoids the race.
import msal # noqa: F401
import requests # noqa: F401

from azure.cli.core._environment import get_config_dir
Comment thread src/azure-cli-core/azure/cli/core/tests/test_auth_eager_import.py
"""After importing azure.cli.core.auth.identity, requests must already be
present in sys.modules so that no background thread can trigger a lazy
import that would race with Python 3.14 per-module import locks."""
import azure.cli.core.auth.identity # noqa: F401
@x-engineering-agent

x-engineering-agent Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Live test skipped

⏭️ Skipping the live test for this revision because the only test file(s) changed are azure-cli-core unit tests, which the live-test pipeline (azdev test --live) does not run — it covers command-module and extension tests only.

These azure-cli-core tests are exercised by upstream CI's unit-test jobs instead. This is informational; no action is required.

@yonzhan

Yong Zhang (yonzhan) commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Core

x-engineering-agent[bot]
x-engineering-agent Bot reviewed Aug 27, 2026
View reviewed changes

@x-engineering-agent x-engineering-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aditya Pujara (@a0x1ab)

Upstream CI

  • azdev-style (7 retries, all failing on the same head d8abdad): run — pylint fails with: 
    src/azure-cli-core/azure/cli/core/auth/identity.py:16:0: W0611: Unused import msal (unused-import)
src/azure-cli-core/azure/cli/core/auth/identity.py:17:0: W0611: Unused import requests (unused-import)
    This is PR-related: pylint does not honor the # noqa: F401 suppression used on the new eager import msal / import requests lines (that comment only suppresses flake8). Next action: replace # noqa: F401 with # pylint: disable=unused-import # noqa: F401 (or add a matching # pylint: disable=unused-import line) on both new import lines in src/azure-cli-core/azure/cli/core/auth/identity.py, then re-run azdev style locally (azdev style azure-cli-core) to confirm pylint passes before re-pushing.
  • Three checks (Azure.azure-cli Breaking Change Test, Azure.azure-cli Full Test, Azure.azure-cli) have been queued for over 3 hours with no re-triggerable check suite found; treating them as abandoned rather than blocking this review. Recommend re-queuing them once the pylint fix above is pushed.

Test validation

  • Live test: skipped — the PR only changes azure-cli-core unit tests, which are not runnable by azdev test --live.
  • Regression coverage: not applicable; the change is accompanied by a focused new unit test (test_auth_eager_import.py) that evicts and re-imports the module to assert requests, requests.structures and msal land in sys.modules.

Risk assessment

  • Scope: single focused change to identity.py plus a matching regression test; no unrelated files touched.
  • Recommended review owner: Core squad (azure-cli-core / auth).

x-engineering-agent[bot]
x-engineering-agent Bot reviewed Aug 27, 2026
View reviewed changes

@x-engineering-agent x-engineering-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aditya Pujara (@a0x1ab)

Upstream CI

  • azdev-style (pylint) — failing, PR-related. The style/pylint job fails with: 
    src/azure-cli-core/azure/cli/core/auth/identity.py:16:0: W0611: Unused import msal (unused-import)
src/azure-cli-core/azure/cli/core/auth/identity.py:17:0: W0611: Unused import requests (unused-import)
    This is caused directly by this PR's eager-import change in identity.py (adding top-level import msal / import requests to force module loading at CLI startup, per the PR description). Pylint flags them as unused because the names aren't referenced elsewhere in the module.
    • Next action: Keep the eager-import behavior but silence the specific warning, e.g. add # pylint: disable=unused-import on those two import lines (or reference the modules, e.g. via importlib.import_module("msal") / importlib.import_module("requests") if a no-op reference is preferred), then re-run azdev style locally before pushing.
    • Verify: Re-run the azdev-style check on the new head; confirm ERROR: Pylint: FAILED no longer appears and Your code has been rated at 10.00/10 remains true.

Test validation

  • Live test: Skipped — no runnable changed test files (only azure-cli-core unit tests changed, not covered by azdev test --live).
  • Regression coverage: Not applicable to the changed files.

Risk assessment

0/100 · Low · Low confidence

The Low rating reflects the detected change scope with no elevated security, reliability, customer, operational, dependency, sovereign-cloud, generated-output, or cross-component signal.

  • Change scope: 2 changed files, 88 changed lines (+88 / -0), including 0 production files.
  • Affected components: No production component was identified.
  • Risk drivers: No elevated risk signal was detected.
  • Regression evidence: No production-code regression-test signal applies.
  • Confidence: Low because no production changed-line evidence was available.
  • Required review: No additional owning-squad review signal was detected.

@x-engineering-agent

x-engineering-agent Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Started a Copilot task using claude-sonnet-4.6 for the automated review at #33997 (review): https://github.com/Azure/azure-cli/tasks/0dfe6fea-47dd-4eb6-ac59-55d5d5439cc6

Copilot AI mentioned this pull request Aug 27, 2026
Draft
3 tasks
x-engineering-agent[bot]
x-engineering-agent Bot reviewed Aug 27, 2026
View reviewed changes

@x-engineering-agent x-engineering-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aditya Pujara (@a0x1ab)

Upstream CI

azdev-style

  • Result: failure
  • Relevance: Uncertain
  • Evidence: The check completed without a concise diagnostic.
  • Suggested fix: Open the linked log and confirm whether the first actionable error touches the PR diff before changing code.
  • Verify: Re-run azdev-style and confirm it passes on the new head.

azdev-style

  • Result: failure
  • Relevance: Uncertain
  • Evidence: The check completed without a concise diagnostic.
  • Suggested fix: Open the linked log and confirm whether the first actionable error touches the PR diff before changing code.
  • Verify: Re-run azdev-style and confirm it passes on the new head.

azdev-style

  • Result: failure
  • Relevance: Uncertain
  • Evidence: The check completed without a concise diagnostic.
  • Suggested fix: Open the linked log and confirm whether the first actionable error touches the PR diff before changing code.
  • Verify: Re-run azdev-style and confirm it passes on the new head.

azdev-style

  • Result: failure
  • Relevance: Uncertain
  • Evidence: The check completed without a concise diagnostic.
  • Suggested fix: Open the linked log and confirm whether the first actionable error touches the PR diff before changing code.
  • Verify: Re-run azdev-style and confirm it passes on the new head.

azdev-style

  • Result: failure
  • Relevance: Uncertain
  • Evidence: The check completed without a concise diagnostic.
  • Suggested fix: Open the linked log and confirm whether the first actionable error touches the PR diff before changing code.
  • Verify: Re-run azdev-style and confirm it passes on the new head.

azdev-style

  • Result: failure
  • Relevance: Uncertain
  • Evidence: The check completed without a concise diagnostic.
  • Suggested fix: Open the linked log and confirm whether the first actionable error touches the PR diff before changing code.
  • Verify: Re-run azdev-style and confirm it passes on the new head.

azdev-style

  • Result: failure
  • Relevance: Uncertain
  • Evidence: The check completed without a concise diagnostic.
  • Suggested fix: Open the linked log and confirm whether the first actionable error touches the PR diff before changing code.
  • Verify: Re-run azdev-style and confirm it passes on the new head.

azdev-style

  • Result: failure
  • Relevance: Uncertain
  • Evidence: The check completed without a concise diagnostic.
  • Suggested fix: Open the linked log and confirm whether the first actionable error touches the PR diff before changing code.
  • Verify: Re-run azdev-style and confirm it passes on the new head.

azdev-style

  • Result: failure
  • Relevance: Uncertain
  • Evidence: The check completed without a concise diagnostic.
  • Suggested fix: Open the linked log and confirm whether the first actionable error touches the PR diff before changing code.
  • Verify: Re-run azdev-style and confirm it passes on the new head.

azdev-style

  • Result: failure
  • Relevance: Uncertain
  • Evidence: The check completed without a concise diagnostic.
  • Suggested fix: Open the linked log and confirm whether the first actionable error touches the PR diff before changing code.
  • Verify: Re-run azdev-style and confirm it passes on the new head.

Test validation

  • Live test: Skipped: live tests are disabled for this repository; upstream CI is authoritative.
  • Regression coverage: Not applicable to the changed files.

Risk assessment

0/100 · Low · Low confidence

The Low rating reflects the detected change scope with no elevated security, reliability, customer, operational, dependency, sovereign-cloud, generated-output, or cross-component signal.

  • Change scope: 2 changed files, 88 changed lines (+88 / -0), including 0 production files.
  • Affected components: No production component was identified.
  • Risk drivers: No elevated risk signal was detected.
  • Regression evidence: No production-code regression-test signal applies.
  • Confidence: Low because no production changed-line evidence was available.
  • Required review: No additional owning-squad review signal was detected.

Copilot AI mentioned this pull request Aug 27, 2026
Draft
3 tasks
@x-engineering-agent

x-engineering-agent Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Started a Copilot task using claude-sonnet-4.6 for the automated review at #33997 (review): https://github.com/a0x1ab/azure-cli/tasks/3d738dbe-315d-4a82-8bff-70d62d106ae1

x-engineering-agent[bot]
x-engineering-agent Bot reviewed Aug 27, 2026
View reviewed changes

@x-engineering-agent x-engineering-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aditya Pujara (@a0x1ab)

CI Failure: azdev-style — Unused Imports in identity.py

Upstream CI

azdev-style

  • Result: failure
  • Relevance: PR-related
  • Evidence (from run log): 
    ERROR: ************* Module azure.cli.core.auth.identity
src/azure-cli-core/azure/cli/core/auth/identity.py:16:0: W0611: Unused import msal (unused-import)
src/azure-cli-core/azure/cli/core/auth/identity.py:17:0: W0611: Unused import requests (unused-import)
ERROR: Pylint: FAILED
  • Cause: The PR adds import msal and import requests at module level in identity.py to perform eager loading at CLI startup and avoid a Python 3.14 module-lock deadlock. Pylint flags both as unused because they are never referenced in that module.
  • Required fix: Suppress the unused-import warnings for these intentional eager-import lines. Add # pylint: disable=unused-import (or inline # noqa: F401) to each import line, or add a brief comment explaining the eager-loading intent so the suppression is self-documenting. For example: 
    import msal  # pylint: disable=unused-import  # eager load to avoid Python 3.14 module-lock deadlock
import requests  # pylint: disable=unused-import  # eager load to avoid Python 3.14 module-lock deadlock
  • Verify: Re-run azdev style locally (azdev style -m azure-cli-core) and confirm it passes with exit code 0.

Test validation

  • Live test: Skipped: no runnable changed test files were found.
  • Regression coverage: Not applicable to the changed files.

Risk assessment

0/100 · Low · Low confidence

The Low rating reflects the detected change scope with no elevated security, reliability, customer, operational, dependency, sovereign-cloud, generated-output, or cross-component signal.

  • Change scope: 2 changed files, 88 changed lines (+88 / -0), including 0 production files.
  • Affected components: No production component was identified.
  • Risk drivers: No elevated risk signal was detected.
  • Regression evidence: No production-code regression-test signal applies.
  • Confidence: Low because no production changed-line evidence was available.
  • Required review: No additional owning-squad review signal was detected.

@x-engineering-agent

x-engineering-agent Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Started a Copilot task using claude-sonnet-4.6 for the automated review at #33997 (review): https://github.com/a0x1ab/azure-cli/tasks/5d314b54-e24d-41dd-aab5-ebb9674d35fa

@a0x1ab
fix: suppress pylint unused-import warnings for eager-load imports in…
e2ae3aa 
… identity.py

Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
@x-engineering-agent

x-engineering-agent Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Live test skipped

⏭️ Skipping the live test for this revision because the only test file(s) changed are azure-cli-core unit tests, which the live-test pipeline (azdev test --live) does not run — it covers command-module and extension tests only.

These azure-cli-core tests are exercised by upstream CI's unit-test jobs instead. This is informational; no action is required.

x-engineering-agent[bot]
x-engineering-agent Bot reviewed Aug 27, 2026
View reviewed changes

@x-engineering-agent x-engineering-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aditya Pujara (@a0x1ab)

Automated review handoff

This PR has reached the automated Copilot fix-iteration cap (3 attempts) and is still showing a failing check, so it is being handed off for human review rather than re-triggering another automated attempt.

Outstanding CI result

Azure.azure-cli

  • Result: failure
  • Failure details:
    • Verify latest index assets — classified Not PR-related
      • Evidence: The job running on agent pool-ubuntu-2204 21 ran longer than the maximum time of 20 minutes.
      • Next action: Re-run the timed-out job. If it times out again at the same setup/checkout step, escalate to the pipeline owner rather than changing PR source — the failure is a job timeout unrelated to the change (an eager import requests/msal addition plus a new unit test in azure-cli-core).
      • Verify: Re-run Verify latest index assets and confirm it completes within the 20-minute limit.

Test validation

  • Live test: Skipped — no runnable changed test files were found (only azure-cli-core unit tests changed).
  • Regression coverage: Not applicable to the changed files.

Review-skill findings

No deterministic or evidence-confirmed semantic findings.

Risk assessment

0/100 · Low · Low confidence

The Low rating reflects the detected change scope with no elevated security, reliability, customer, operational, dependency, sovereign-cloud, generated-output, or cross-component signal.

  • Change scope: 2 changed files, 88 changed lines (+88 / -0), including 0 production files.
  • Affected components: No production component was identified.
  • Risk drivers: No elevated risk signal was detected.
  • Regression evidence: No production-code regression-test signal applies.
  • Confidence: Low because no production changed-line evidence was available.
  • Required review: No additional owning-squad review signal was detected.

@a0x1ab Aditya Pujara (a0x1ab) added X Engineering Agent Reviewed Pull request reviewed by X Engineering Agent and removed azure-client-tools-agent labels Sep 2, 2026
@microsoft-github-policy-service

microsoft-github-policy-service Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🔔 Routing this PR to @Azure/act-quality-productivity-squad.

@microsoft-github-policy-service

microsoft-github-policy-service Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🔔 Routing this PR to @Azure/act-platform-engineering-squad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@x-engineering-agent x-engineering-agent[bot] x-engineering-agent[bot] left review comments
@yonzhan Yong Zhang (yonzhan) Awaiting requested review from yonzhan
@jsntcy Yu Chen (jsntcy) Awaiting requested review from jsntcy
@necusjz Ethan Yang (necusjz) Awaiting requested review from necusjz

At least 1 approving review is required to merge this pull request.

Assignees

@necusjz Ethan Yang (necusjz)

Labels

act-platform-engineering-squad act-quality-productivity-squad Auto-Assign Auto assign by bot Core CLI core infrastructure Network az network vnet/lb/nic/dns/etc... X Engineering Agent Reviewed Pull request reviewed by X Engineering Agent

Projects

None yet

Milestone

October 2026 (2026-10-06)

Development

Successfully merging this pull request may close these issues.

az network vnet create intermittently fails with Python 3.14 _ModuleLock deadlock while importing requests.structures

5 participants

@a0x1ab @yonzhan @necusjz