[AppService] Restore appServicePlanId in az webapp show/list output - #33733
Aditya Pujara (a0x1ab) with Copilot wants to merge 14 commits into
Conversation
|
Validation for Azure CLI Full Test Starting...
Thanks for your contribution! |
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
appServicePlanId in az webapp show/list output
Live test results —
|
There was a problem hiding this comment.
Automated Review Summary
Live test: ✅ Passed
CI checks: ✅ 1/1 passed, 0 pending, 0 failed
All automated checks are green for this PR at the current head commit. No further action needed from the agent at this time.
Posted by agent-assist (autonomous bug-fix pipeline).
|
AppService |
There was a problem hiding this comment.
CI Failure Summary
The following checks failed on this PR:
❌ Failed Checks
Azure.azure-cli (Test Homebrew Formula)— The Homebrew formula test suite failed.Azure.azure-cli— The overall Azure DevOps pipeline check failed (rolls up the Homebrew failure).
Root Cause Analysis
The PR adds test_rename_server_farm_props_handles_mutable_mapping which sets site['serverFarmId'] = ... directly on a Site model instance from azure.mgmt.web.models. The Homebrew formula test environment pins a specific version of azure-mgmt-web; in that pinned version, Site objects may not support dict-style item assignment (__setitem__), causing either a TypeError on site['serverFarmId'] = ... or causing isinstance(site, MutableMapping) to return False, which would make _rename_server_farm_props fall through to the setattr branch instead of the dict branch — and the test assertion site['appServicePlanId'] would then fail.
Required Fix
-
Decouple the test from the real
SiteSDK model. Replace theSite(location='westus')instance with a plainMutableMappingmock (e.g., a simpledict) so the test exercises_rename_server_farm_propswithout depending on the SDK version installed in the Homebrew formula environment.# Before (SDK-version-dependent) site = Site(location='westus') site['serverFarmId'] = '...' # After (SDK-independent) site = {'serverFarmId': '...', 'location': 'westus'}
-
Remove the now-unused
from azure.mgmt.web.models import Siteimport to eliminate the environment-version risk. -
Verify locally that
python -m pytest --pyargs azure.cli.command_modules.appservicepasses before pushing.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Copilot the automated CI and live-test review on this PR found failures. Please look into the requested changes and push a fix. Posted by agent-assist (autonomous bug-fix pipeline). |
Fixed in |
Live test results —
|
There was a problem hiding this comment.
CI Failure Summary
This PR's Azure DevOps validation build is failing across the board (attempt 2 of 3 automated cycles):
- Azure.azure-cli (main build) — Build #20260819.12 failed
- Azure.azure-cli Full Test — test failures in the main run plus:
- Automation Full Test Python312 Profile Latest (instance3)
- Automation Full Test Python314 Profile Latest (instance3)
- Package tests — failing for essentially every target:
- Deb: Jammy AMD64/ARM64, Bookworm AMD64/ARM64, Noble AMD64/ARM64, Bullseye AMD64/ARM64
- Rpm: Red Hat UBI 8/9/10 AMD64/ARM64, Azure Linux 3.0/4.0(Beta) AMD64/ARM64
Given the breadth (main build + full test + every packaged distro), this is most
likely a build-breaking regression introduced by this change (e.g. a syntax/import
error or a test that fails at collection time) rather than an environment flake.
Please:
- Check the failing "Azure.azure-cli" build log first — a top-level build failure
usually explains the cascading package/test failures. - Fix the root cause and push a commit to this PR branch.
- Re-run/verify locally with
azdev testfor the AppService module before pushing.
PR: #33733
|
Started a Copilot task using |
… compatibility The transformer was removing serverFarmId from the output which broke existing live tests in test_logicapp_commands.py and test_functionapp_commands.py that explicitly check for serverFarmId in the output. Fix by using get() instead of pop() so that serverFarmId is preserved while appServicePlanId is added as an alias. Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Fixed in the latest commit. Root cause: The transformer was using Fix: Changed |
Live test results —
|
There was a problem hiding this comment.
CI Status: Failures detected
Azure DevOps validation build failed for the latest commit (ec3a6fbe3db1fa5daf33cb0defa8ce65a1624de0):
- Azure.azure-cli — Build #20260819.23 failed (9 errors / 4 warnings). See the build log.
- Azure.azure-cli (Test Rpm Package Azure Linux 3.0 AMD64) — failed (1 error). See the build log.
The dispatched live-test workflow passed, but the main validation build itself is failing with 9 build errors. Please inspect the Azure DevOps build log linked above, fix the underlying issues (likely a syntax/import/lint error introduced by this change, and/or an RPM packaging test regression), and push a new commit.
|
Started a Copilot task using |
Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Live test results —
|
Live test results —
|
There was a problem hiding this comment.
Automated Review Summary — PASS ✅
Live test: Reused a prior successful dispatch of azdev test against the changed test files for this PR — result: success.
CI checks: All 20 checks completed and passed (0 failed, 0 pending).
No further action needed from the automation side. A human maintainer can proceed with final review/merge at their discretion.
|
🔔 Routing this PR to @Azure/act-observability-squad. |
Shreyas Gopalakrishna (shreyas-gopalakrishna)
left a comment
There was a problem hiding this comment.
Aditya Pujara (@a0x1ab) changes here look good, could you rebase and get this merged
|
Yong Zhang (@yonzhan) requesting your review as well to get this out sooner since we have multiple issues raised for this bug |
|
Please fix CI issues |
🤖 PR Validation — ❌ Action needed
❌AzureCLI-BreakingChangeTest
Please submit your Breaking Change Pre-announcement ASAP if you haven't already. Please note:
Related command
az webapp listaz webapp showDescription
az webapp listandaz webapp showregressed to returningserverFarmIdinstead of the long-standingappServicePlanIdfield after the newer Web SDK model shape was introduced. This change restores the compatibility shim so CLI output keeps the expected contract.Output compatibility
_rename_server_farm_propsto handle newer mutable-mapping SDK models by writingappServicePlanIddirectly into the serialized shape.serverFarmIdfrom that output path soshow/listmatch prior CLI behavior.Legacy path preserved
Regression coverage
Testing Guide
Expected: both commands return
appServicePlanId;serverFarmIdis no longer surfaced in the CLI output contract for these commands.History Notes
[AppService]
az webapp show/list: RestoreappServicePlanIdoutput field compatibilityThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.