{App Service} az webapp deploy: cache SCM URL and headers within a …#33560
Open
naveedaz wants to merge 1 commit into
Open
{App Service} az webapp deploy: cache SCM URL and headers within a …#33560naveedaz wants to merge 1 commit into
az webapp deploy: cache SCM URL and headers within a …#33560naveedaz wants to merge 1 commit into
Conversation
️✔️AzureCLI-FullTest
|
❌AzureCLI-BreakingChangeTest
Please submit your Breaking Change Pre-announcement ASAP if you haven't already. Please note:
|
microsoft-github-policy-service
Bot
requested review from
jsntcy,
wangzelin007,
yanzhudd,
yonzhan and
zhoxing-ms
microsoft-github-policy-service
Bot
added
Web Apps
naveedaz
force-pushed
the
fix/appservice-cache-scm-headers-onedeploy
branch
from
2028829 to
c5d1a42
Compare
Collaborator
|
App Service |
naveedaz
force-pushed
the
fix/appservice-cache-scm-headers-onedeploy
branch
2 times, most recently
from
ff243b4 to
bf34e57
Compare
…single deploy invocation `az webapp deploy` currently issues ~12 ARM calls per invocation. Most of the duplication comes from two helpers that are called twice each within a single deploy: * `_get_scm_url` is called from `_build_onedeploy_scm_url` (publish URL) and `_get_onedeploy_status_url` (status URL). The SCM hostname does not change between the two — 1 ARM call (`GET /sites`) is wasted. * `get_scm_site_headers` is called from `_get_ondeploy_headers` (publish POST) and again ~20–60 s later from `_check_zip_deployment_status` (status poll). Each invocation runs `is_flex_functionapp` + `basic_auth_supported` + `_get_site_credential` — 3 wasted ARM calls. Hoist both results onto `OneDeployParams` for the lifetime of one deploy, reuse them on the status leg, and clear them in a `finally` block so a basic-auth credential cannot outlive the invocation (and cannot be observed by outer exception handlers or telemetry). On a HTTP 401 from the poll we drop the cache and refetch once to handle credential rotation. Net effect per invocation: 12 → 7 ARM calls (–42%), and the `_get_site_credential` exposure (the call that has been observed to surface sporadic `ConnectionResetError(10054)`) drops from 2 to 1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
naveedaz
force-pushed
the
fix/appservice-cache-scm-headers-onedeploy
branch
from
bf34e57 to
c79a930
Compare
naveedaz
requested review from
DanielMicrosoft,
bebound and
necusjz
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR introduces per-invocation caching for OneDeploy to reduce redundant ARM calls during az webapp deploy / az functionapp deploy, while adding targeted unit tests and documenting the change.
Changes:
- Cache the Site model, derived SCM URL, and stable SCM auth headers on
OneDeployParamsfor the duration of a deploy invocation. - Thread cached parameters through status polling helpers to reuse headers/site and refresh credentials on 401.
- Add comprehensive mock-based tests and update release notes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/azure-cli/azure/cli/command_modules/appservice/custom.py | Implements Site/SCM URL/SCM header caching and updates OneDeploy/status polling flow to reuse cached data. |
| src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py | Adds unit tests validating cache behavior, 401 refresh, and non-leakage of credentials. |
| src/azure-cli/HISTORY.rst | Documents the reduced ARM-call behavior for deploy commands. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+11460
to
+11462
| headers = get_scm_site_headers( | ||
| params.cmd.cli_ctx, params.webapp_name, params.resource_group_name, | ||
| is_flex_hint=_known_is_flex_hint(params)) |
Comment on lines
+11026
to
+11030
| # Eagerly fetch (and cache) the Site so downstream helpers | ||
| # (_get_or_fetch_scm_url, _get_visit_url, _get_or_fetch_is_linux_webapp) | ||
| # can derive answers from the cached model without issuing additional | ||
| # GET /sites calls. Also derive the is_flex hint here so | ||
| # get_scm_site_headers can skip the redundant is_flex_functionapp call. |
Comment on lines
+9763
to
+9765
| if deploy_params is not None and deploy_params._cached_scm_headers is not None: # pylint: disable=protected-access | ||
| headers = dict(deploy_params._cached_scm_headers) # pylint: disable=protected-access | ||
| headers['x-ms-client-request-id'] = cmd.cli_ctx.data['headers']['x-ms-client-request-id'] |
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.
Related command
az webapp deploy,az functionapp deployDescription
az webapp deployandaz functionapp deployissue ~12 redundant ARM calls per invocation. Most duplication comes from helpers that independently fetch the same Site model, SCM URL, publishing credentials, andis_flexstatus:_get_scm_urlfetches GET /sites to resolve the SCM hostname — called twice (publish URL + status URL).get_scm_site_headersrunsis_flex_functionapp+basic_auth_supported+_get_site_credential— called twice (publish POST + status poll), wasting 3+ ARM calls each time.This PR hoists per-invocation state onto
OneDeployParams:_cached_site. Derive SCM URL (fromhost_name_ssl_states),is_linux(fromreserved),is_flex(fromsku), and visit URL (fromenabled_host_names) — zero additional ARM calls.get_scm_site_headersresult on_cached_scm_headers. Reused by status poller, Kudu warmup, and_get_latest_deployment_id. On HTTP 401 the cache is dropped and headers are refetched once._cached_scm_headersis cleared in afinallyblock so basic-auth credentials cannot outlive the invocation.urllib3.util.make_headers(basic_auth=...)returns lowercase'authorization'; AAD uses'Authorization'. Cache population handles both via case-insensitive key matching.ARM call reduction (verified via E2E)
Testing Guide
Deploy with
--debugand countRequest URL:.*management.azure.comlines (excludingdeploymentStatuspolling):az webapp deploy -g <rg> -n <app> --src-path <zip> --type zip --debugUnit tests: 160 tests covering cache lifecycle, SCM URL derivation from
host_name_ssl_states, slot awareness,is_flexhint from cached SKU, header caching, and cache cleanup on success/exception.History Notes
[App Service]
az webapp deploy/az functionapp deploy: Reduce per-deploy ARM calls from ~12 to ~3 by caching the Site model, SCM URL, and SCM authentication headers for the lifetime of a single deploy invocation