-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[confcom] bugfix for json input being case-insensitive #8895
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
Conversation
️✔️Azure CLI Extensions Breaking Change Test
|
|
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>
|
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 restores case-insensitive handling of JSON input fields in confcom, bumps the extension version to 1.2.6, and updates tests to align with the new behavior.
- Extended
case_insensitive_dict_getto accept adefault_valueand updated all field lookups to use it. - Bumped version in
setup.py,internal_config.json, and added an entry inHISTORY.rst. - Updated scenario and conversion tests to reflect new expected values and volume-mount behavior.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/confcom/setup.py | Version bumped to 1.2.6 |
| src/confcom/azext_confcom/tests/latest/test_confcom_scenario.py | Updated expected image layer digests |
| src/confcom/azext_confcom/tests/latest/test_confcom_policy_conversion.py | Added an emptyDir mount case and corresponding assertions |
| src/confcom/azext_confcom/template_util.py | Added default_value to case_insensitive_dict_get and updated call sites |
| src/confcom/azext_confcom/data/internal_config.json | Version bumped to 1.2.6 |
| src/confcom/HISTORY.rst | Added 1.2.6 entry for bugfix |
Comments suppressed due to low confidence (2)
src/confcom/azext_confcom/template_util.py:1640
- [nitpick] Using
ACI_FIELD_CONTAINERS_ENVS_NAMEfor a mount's name can be confusing. Consider defining or using a dedicated constant likeACI_FIELD_CONTAINERS_MOUNTS_NAMEto clarify that this field refers to the mount name.
config.ACI_FIELD_CONTAINERS_ENVS_NAME: mount_name,
src/confcom/azext_confcom/tests/latest/test_confcom_policy_conversion.py:119
- Add unit tests for
case_insensitive_dict_getitself—e.g., verify it returns the provideddefault_valuewhen a key is absent and handles different casings correctly—so its behavior is explicitly covered.
self.assertFalse(vm[cfg.ACI_FIELD_TEMPLATE_MOUNTS_READONLY])
|
|
@jsntcy Can you please review this soon as this issue is blocking the partners. |
|
[Release] Update index.json for extension [ confcom ] : https://dev.azure.com/msazure/One/_build/results?buildId=128574130&view=results |
Fixing case sensitivity for fields in json
Fields are typically case-insensitive in confcom. In #8835 a bug was introduced where the input.json format was no longer case-insensitive. This PR fixes that.
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az confcom
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install wheel==0.30.0required)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.