-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[App Service] Fix #32290: az functionapp config appsettings set --slot-settings: Fix the command failure to update existing slot settings
#32291
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
[App Service] Fix #32290: az functionapp config appsettings set --slot-settings: Fix the command failure to update existing slot settings
#32291
Conversation
Ensures slot settings passed in object format are properly marked and processed before applying application settings. Moves slot configuration processing earlier to guarantee correct configuration order and maintain expected behavior, especially when mixing regular and slot-specific settings. Relocates workaround for Centauri Function Apps to follow updated logic flow for improved clarity.
️✔️AzureCLI-FullTest
|
|
Hi @jcassanji-southworks, |
️✔️AzureCLI-BreakingChangeTest
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
|
Thank you for your contribution @jcassanji-southworks! We will review the pull request and get back to you soon. |
… tests for validation
@microsoft-github-policy-service agree company=southworks |
There was a problem hiding this comment.
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 fixes issue #32290 where az functionapp config appsettings set --slot-settings failed to update existing slot settings, and modernizes error handling by replacing deprecated CLIError with structured error types following Azure CLI Error Handling Guidelines.
Key Changes:
- Fixed slot settings processing order to handle configurations before applying application settings
- Replaced
CLIErrorwith appropriate structured error types (MutuallyExclusiveArgumentError,InvalidArgumentValueError,AzureResponseError) - Added comprehensive unit tests (5 new tests) to validate error handling and slot settings functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| custom.py | Fixed slot settings logic, replaced deprecated CLIError with structured error types, improved error messages with actionable recommendations |
| test_webapp_commands_thru_mock.py | Added 5 unit tests for error handling scenarios and slot settings functionality |
Comments suppressed due to low confidence (1)
src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py:1
- The test
test_update_app_settings_error_handling_invalid_formatdoes not cover the ValueError exception path on lines 563-567. The test mocksshell_safe_json_parseto raiseInvalidArgumentValueErrordirectly, which triggers the outer exception handler before reaching thesplit('=', 1)logic. Add a test case whereshell_safe_json_parseraisesInvalidArgumentValueErrorAND the string doesn't contain '=' to properly test this ValueError path.
# --------------------------------------------------------------------------------------------
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…format without equals sign
…ate-existing-function-app-slot-settings-fix' of https://github.com/jcassanji-southworks/azure-cli into 32290-function-app-slot-settings-parameter-fails-to-update-existing-function-app-slot-settings-fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...zure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py
Outdated
Show resolved
Hide resolved
...zure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py
Outdated
Show resolved
Hide resolved
…enhance error handling in tests
…ling JSON objects and expected error messages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…prove error messaging for invalid formats
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...zure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py
Outdated
Show resolved
Hide resolved
...zure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py
Outdated
Show resolved
Hide resolved
...zure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py
Outdated
Show resolved
Hide resolved
...zure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py
Outdated
Show resolved
Hide resolved
...zure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py
Outdated
Show resolved
Hide resolved
...zure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py
Outdated
Show resolved
Hide resolved
…t/test_webapp_commands_thru_mock.py Co-authored-by: Copilot <[email protected]>
…t/test_webapp_commands_thru_mock.py Co-authored-by: Copilot <[email protected]>
…t/test_webapp_commands_thru_mock.py Co-authored-by: Copilot <[email protected]>
…t/test_webapp_commands_thru_mock.py Co-authored-by: Copilot <[email protected]>
…t/test_webapp_commands_thru_mock.py Co-authored-by: Copilot <[email protected]>
…t/test_webapp_commands_thru_mock.py Co-authored-by: Copilot <[email protected]>
…e and JSON settings, improving clarity and error handling
…ate-existing-function-app-slot-settings-fix' of https://github.com/jcassanji-southworks/azure-cli into 32290-function-app-slot-settings-parameter-fails-to-update-existing-function-app-slot-settings-fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…ing code readability
…ate-existing-function-app-slot-settings-fix' of https://github.com/jcassanji-southworks/azure-cli into 32290-function-app-slot-settings-parameter-fails-to-update-existing-function-app-slot-settings-fix
…function signature
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
az functionapp config appsettings set --slot-settings: Fix the command failure to update existing slot settings
|
please involve others responsible for the business logic to review this PR as well, since Azure CLI is only responsible for code style and specification. |
|
Tagging the business logic owners for visibility: |
| return False | ||
|
|
||
|
|
||
| def _parse_fallback_key_value_setting(s, result): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function does (almost?) the same thing as _parse_simple_key_value_setting in line 514. It would never succeed if _parse_simple_key_value_setting fails.
| continue | ||
|
|
||
| # Fallback to key=value parsing with error handling | ||
| _parse_fallback_key_value_setting(s, result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only called if _parse_simple_key_value_setting in line 605 returns false. But then this call will also not succeed, except if there is a problem with slot_result, as this function takes result as a parameter instead of slot_result for a slot setting.
[App Service] Fix #32290: az functionapp config appsettings set --slot-settings fails to update existing slot settings + Azure CLI Error Handling Guidelines Implementation
📋 Summary
This PR addresses issue #32290 where
az functionapp config appsettings set --slot-settingsfails to update existing function app slot settings. Additionally, this PR implements the Azure CLI Error Handling Guidelines by replacing deprecatedCLIErrorusage with structured error types, and incorporates code quality improvements based on Copilot AI recommendations.🐛 Issue Fixed
Problem: The
--slot-settingsparameter inaz functionapp config appsettings setcommand was not properly updating existing slot settings configuration.Root Cause: The slot settings logic in
update_app_settings()function processed application settings values BEFORE configuring which settings should be slot-specific, causing existing slot settings to lose their slot configuration.🔧 Solution Implemented
Core Functionality Fix
--slot-settingswith JSON object formatAzure CLI Error Handling Guidelines Implementation
Following the mandatory Azure CLI Error Handling Guidelines, this PR modernizes error handling by replacing deprecated
CLIErrorwith appropriate structured error types:🔄 Error Type Replacements
CLIError→MutuallyExclusiveArgumentErrorupdate_app_settings()parameter validation--settingsnor--slot-settingsare provided"Please provide either --settings or --slot-settings parameter."CLIError→InvalidArgumentValueErrorupdate_app_settings()setting format parsing"Invalid setting format: '{value}'. Expected 'key=value' format or valid JSON.""Use 'key=value' format or provide valid JSON like '{\"key\": \"value\"}'."CLIError→AzureResponseErrorupdate_application_settings_polling()error handling"Failed to update application settings: {error_details}"Code Quality Improvements (Copilot AI Recommendations)
Based on Copilot AI code review feedback, the following improvements were implemented:
🧪 Test Optimization
🔧 Code Clarity Improvements
else+ifstructure to clearif/elifpattern🧪 Testing
Comprehensive Unit Test Suite (6 tests)
Added comprehensive unit tests in
test_webapp_commands_thru_mock.pyfollowing Azure CLI testing patterns:test_update_app_settings_error_handling_no_parametersMutuallyExclusiveArgumentErrorwhen no parameters providedtest_update_app_settings_error_handling_invalid_formatInvalidArgumentValueErrorfor invalid setting formatstest_update_app_settings_error_handling_invalid_format_no_equalsValueErrorpath when string contains no '=' and JSON parsing failstest_update_app_settings_success_key_value_formatkey=valueformat settingstest_update_application_settings_polling_error_handlingAzureResponseErrorin polling functiontest_update_app_settings_success_with_slot_settingsTest Results ✅
📁 Files Changed
Core Implementation
src/azure-cli/azure/cli/command_modules/appservice/custom.pyupdate_app_settings()CLIErrorwith appropriate structured error typesupdate_application_settings_polling()error handlingTesting
src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py🔍 Impact Assessment
✅ Backward Compatibility
🎯 User Experience Improvements
📊 Quality Metrics
🤖 AI-Assisted Quality Assurance
🚀 Benefits
📋 Pre-Submission Checklist
🎯 Related Issues