Repository navigation
[App Config] az appconfig enhanced-feature-flag: Add command group for enhanced feature flags - #34043
Christine Wanjau (ChristineWanjau) wants to merge 6 commits into
Conversation
az appconfig enhanced-feature-flag: Add command group for enhanced feature flagsaz appconfig enhanced-feature-flag: Add command group for enhanced feature flags
|
App Config |
There was a problem hiding this comment.
🟡 Changes recommended
The enhanced feature flag serializer returns last_modified without normalizing datetime values, which can break downstream formatting/output handling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new az appconfig enhanced-feature-flag command group to manage “enhanced feature flags” via the App Configuration data-plane FeatureFlagClient, wiring it into the App Config module’s command table, args, help, output formatting, and scenario tests.
Changes:
- Introduces
appconfig enhanced-feature-flagcommands (set/show/list/delete/enable/disable) backed byFeatureFlagClient. - Extends App Config command plumbing: new args/validators, help text, output formatting, and constants.
- Adds a new scenario test + recording; refactors shared data-plane client auth helper for
--auth-mode login.
File summaries
| File | Description |
|---|---|
| src/azure-cli/HISTORY.rst | Release note entry for the new command group |
| src/azure-cli/azure/cli/command_modules/appconfig/enhanced_feature.py | Implements enhanced feature flag CRUD/enable/disable and serialization |
| src/azure-cli/azure/cli/command_modules/appconfig/commands.py | Registers the new appconfig enhanced-feature-flag command group |
| src/azure-cli/azure/cli/command_modules/appconfig/_params.py | Adds parameters for enhanced feature flags (fields, tags, etc.) |
| src/azure-cli/azure/cli/command_modules/appconfig/_validators.py | Adds validator for enhanced feature --fields |
| src/azure-cli/azure/cli/command_modules/appconfig/_utils.py | Adds FeatureFlagClient factory + refactors login credential resolution |
| src/azure-cli/azure/cli/command_modules/appconfig/_help.py | Adds help + examples for the new commands |
| src/azure-cli/azure/cli/command_modules/appconfig/_format.py | Adds table output formatting for enhanced feature flags |
| src/azure-cli/azure/cli/command_modules/appconfig/_constants.py | Adds constants used in enhanced feature flag serialization |
| src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/test_appconfig_enhanced_feature_commands.py | Adds scenario coverage for enhanced feature flags |
| src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/recordings/test_azconfig_enhanced_feature.yaml | Adds recording for the new scenario test |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def _serialize_feature_flag(feature_flag, fields=None): | ||
| result = { | ||
| FeatureFlagConstants.NAME: feature_flag.name, | ||
| FeatureFlagConstants.ENABLED: feature_flag.enabled, | ||
| FeatureFlagConstants.LABEL: feature_flag.label, | ||
| FeatureFlagConstants.DESCRIPTION: feature_flag.description, | ||
| FeatureFlagConstants.CONDITIONS: feature_flag.conditions.as_dict() if feature_flag.conditions is not None else None, | ||
| FeatureFlagConstants.VARIANTS: [variant.as_dict() for variant in feature_flag.variants] if feature_flag.variants is not None else None, | ||
| FeatureFlagConstants.ALLOCATION: feature_flag.allocation.as_dict() if feature_flag.allocation is not None else None, | ||
| FeatureFlagConstants.TELEMETRY: feature_flag.telemetry.as_dict() if feature_flag.telemetry is not None else None, | ||
| FeatureFlagConstants.TAGS: feature_flag.tags, | ||
| FeatureFlagConstants.LAST_MODIFIED: feature_flag.last_modified, | ||
| } | ||
|
|
||
| if fields: | ||
| return {field: result[field] for field in fields if field in result} | ||
|
|
||
| return result |
Add set/show/list/delete/enable/disable commands backed by the new data-plane FeatureFlagClient. Supports --telemetry-enabled and --fields (including variants and allocation); drops the --key property. Includes help, output formatting, params, a scenario test with recording, and a HISTORY entry.
…d top-level set arguments Support defining the entire enhanced feature flag with --flag using full JSON or shorthand syntax (mirroring the data-plane API schema), and add top-level --enable/--description/--tags/--telemetry-enabled arguments. --flag is a full replacement and is mutually exclusive with the per-property arguments. Re-recorded the scenario test cassette. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
8ccc9dd to
2999399
Compare
…to `--feature-name` and tidy validators/registration Use feature_name as the parameter/dest for the enhanced feature flag name across params, handlers and tests (option is now --feature-name only). Split validation into validate_feature (classic feature commands) and validate_enhanced_feature_flag, and rename the --flag validator to validate_enhanced_feature_flag_input. Register the command group via a dedicated configstore_enhanced_featureflag_util CliCommandType. Update help examples and re-record the scenario test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| - name: List all enhanced feature flags. | ||
| text: | ||
| az appconfig enhanced-feature-flag list -n MyAppConfiguration |
There was a problem hiding this comment.
This shows how to list all enhanced feature flags, but do we show how to list all feature flags without labels. i.e. \0
There was a problem hiding this comment.
Good call — added an example using --label \0 to list flags with null labels.
| - name: Enable an enhanced feature flag using App Configuration store name. | ||
| text: | ||
| az appconfig enhanced-feature-flag enable -n MyAppConfiguration --feature-name color --label test | ||
| - name: Force enabling an enhanced feature flag using connection string. |
There was a problem hiding this comment.
Why do we use the word Force here and in disable. We don't use that word for the other items. Is it because you are also adding in --yes, but we use without confirmation in the other items.
There was a problem hiding this comment.
Used "Force" to match the existing az appconfig feature enable/disable help, which already phrases the --yes example that way (L592) — happy to switch everything to "without confirmation" if you prefer.
Separately: the classic feature module has no --enable on set, only standalone enable/disable. Should I drop --enable from enhanced-feature-flag set to match that pattern, or keep it for convenience? Jimmy Campbell (@jimmyca15)
There was a problem hiding this comment.
I think we should keep the stand alone Enable/Disable, these are meant to be better feature flags.
| "--requirement-type or --telemetry-enabled.") | ||
|
|
||
| with self.argument_context('appconfig enhanced-feature-flag show') as c: | ||
| c.argument('fields', arg_type=enhanced_feature_fields_arg_type) |
There was a problem hiding this comment.
Is there a reason this one doesn't have help?
There was a problem hiding this comment.
The help text is defined on the shared enhanced_feature_fields_arg_type (help='Customize output fields for Enhanced Feature Flags.'), so the fields argument inherits it here rather than specifying help inline.
| if auth_mode == "anonymous": | ||
| try: | ||
| feature_flag_client = FeatureFlagClient( | ||
| base_url=endpoint, | ||
| credential=AzureKeyCredential(key=""), | ||
| id_credential="", | ||
| user_agent=HttpHeaders.USER_AGENT, | ||
| transport=AuthHeaderRequestsTransport(), | ||
| retry_policy=retry_policy) | ||
| except (ValueError, TypeError) as ex: | ||
| raise CLIError("Failed to initialize FeatureFlagClient due to an exception: {}".format(str(ex))) | ||
|
|
||
| if auth_mode == "key": | ||
| connection_string = resolve_connection_string(cmd, name, connection_string) | ||
| try: | ||
| feature_flag_client = FeatureFlagClient.from_connection_string(connection_string=connection_string, | ||
| user_agent=HttpHeaders.USER_AGENT, | ||
| retry_policy=retry_policy) | ||
| except ValueError as ex: | ||
| raise CLIError("Failed to initialize FeatureFlagClient due to an exception: {}".format(str(ex))) | ||
|
|
||
| if auth_mode == "login": | ||
| credential, endpoint = resolve_login_credential(cmd, name, endpoint) | ||
| try: | ||
| feature_flag_client = FeatureFlagClient(credential=credential, | ||
| base_url=endpoint, | ||
| user_agent=HttpHeaders.USER_AGENT, | ||
| retry_policy=retry_policy) | ||
| except (ValueError, TypeError) as ex: | ||
| raise CLIError("Failed to initialize FeatureFlagClient due to an exception: {}".format(str(ex))) |
There was a problem hiding this comment.
| if auth_mode == "anonymous": | |
| try: | |
| feature_flag_client = FeatureFlagClient( | |
| base_url=endpoint, | |
| credential=AzureKeyCredential(key=""), | |
| id_credential="", | |
| user_agent=HttpHeaders.USER_AGENT, | |
| transport=AuthHeaderRequestsTransport(), | |
| retry_policy=retry_policy) | |
| except (ValueError, TypeError) as ex: | |
| raise CLIError("Failed to initialize FeatureFlagClient due to an exception: {}".format(str(ex))) | |
| if auth_mode == "key": | |
| connection_string = resolve_connection_string(cmd, name, connection_string) | |
| try: | |
| feature_flag_client = FeatureFlagClient.from_connection_string(connection_string=connection_string, | |
| user_agent=HttpHeaders.USER_AGENT, | |
| retry_policy=retry_policy) | |
| except ValueError as ex: | |
| raise CLIError("Failed to initialize FeatureFlagClient due to an exception: {}".format(str(ex))) | |
| if auth_mode == "login": | |
| credential, endpoint = resolve_login_credential(cmd, name, endpoint) | |
| try: | |
| feature_flag_client = FeatureFlagClient(credential=credential, | |
| base_url=endpoint, | |
| user_agent=HttpHeaders.USER_AGENT, | |
| retry_policy=retry_policy) | |
| except (ValueError, TypeError) as ex: | |
| raise CLIError("Failed to initialize FeatureFlagClient due to an exception: {}".format(str(ex))) | |
| try: | |
| if auth_mode == "anonymous": | |
| feature_flag_client = FeatureFlagClient( | |
| base_url=endpoint, | |
| credential=AzureKeyCredential(key=""), | |
| id_credential="", | |
| user_agent=HttpHeaders.USER_AGENT, | |
| transport=AuthHeaderRequestsTransport(), | |
| retry_policy=retry_policy) | |
| elif auth_mode == "key": | |
| connection_string = resolve_connection_string(cmd, name, connection_string) | |
| feature_flag_client = FeatureFlagClient.from_connection_string(connection_string=connection_string, | |
| user_agent=HttpHeaders.USER_AGENT, | |
| retry_policy=retry_policy) | |
| elif auth_mode == "login": | |
| credential, endpoint = resolve_login_credential(cmd, name, endpoint) | |
| feature_flag_client = FeatureFlagClient(credential=credential, | |
| base_url=endpoint, | |
| user_agent=HttpHeaders.USER_AGENT, | |
| retry_policy=retry_policy) | |
| except (ValueError, TypeError) as ex: | |
| raise CLIError("Failed to initialize FeatureFlagClient due to an exception: {}".format(str(ex))) |
Is there a reason that we just do multiple ifs and just just have the single except?
There was a problem hiding this comment.
No real reason — collapsed it into a single try/except with if/elif. Thanks!
| if unknown_keys: | ||
| raise InvalidArgumentValueError( | ||
| "Unsupported feature flag properties: {}. Allowed properties: {}.".format( | ||
| ", ".join(sorted(unknown_keys)), | ||
| ", ".join(sorted(FEATURE_FLAG_PROPERTIES)))) |
There was a problem hiding this comment.
Is there a reason we don't allow unknown fields? This is usually how we had new features. We don't block them in the data plane I believe.
There was a problem hiding this comment.
Agreed — dropped the allow-list check so unknown properties flow through to the data plane.
| if feature_flag is None: | ||
| raise CLIErrors.ResourceNotFoundError("Enhanced feature flag '{}' with label '{}' does not exist.".format(feature_name, label)) |
There was a problem hiding this comment.
Why is this here feature flag client can't return None should it just be up in the resource not found error?
There was a problem hiding this comment.
Good point. Updated the delete function so there is no redundant None handling in delete anymore.
| confirmation_message = "Are you sure you want to delete the enhanced feature flag '{}' with label '{}'?".format(feature_name, label) | ||
| user_confirmation(confirmation_message, yes) | ||
|
|
||
| try: | ||
| deleted_feature_flag = feature_flag_client.delete_feature_flag(name=feature_name, label=label) | ||
| except HttpResponseError as exception: | ||
| raise CLIErrors.AzureResponseError(str(exception)) |
There was a problem hiding this comment.
I doubt it is actually an issue, but what happens if a feature flag is deleted while waiting for this conformation. I assume it would raise a weird cli error on line 176?
There was a problem hiding this comment.
Just verified that deletes only happen after user_confirmation, and each one is etag-guarded (If-Match via IfNotModified). So if a flag is removed while we wait, the service returns 412 and we surface it as an a error in line 176. This mirrors az appconfig kv delete and az appconfig feature delete.
|
Please fix CI issues |
- Consolidate FeatureFlagClient auth-mode init into a single try/except - Allow unknown properties in --flag for forward compatibility - Support deleting multiple enhanced feature flags via name/label/tag filters - Add list example for null labels and update delete help/examples - Re-record scenario test for the new multi-delete behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…onsistency with feature set Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Matthew Metcalf <mrm9084@gmail.com>
🤖 PR Validation —⚠️ Review suggested
Related command
az appconfig enhanced-feature-flagDescription
Adds a new
az appconfig enhanced-feature-flagcommand group backed by the new App Configuration data-planeFeatureFlagClient(fromazure-appconfiguration==1.10.0b1).Commands:
az appconfig enhanced-feature-flag set(required:--feature-name)az appconfig enhanced-feature-flag show(required:--feature-name)az appconfig enhanced-feature-flag listaz appconfig enhanced-feature-flag delete(required:--feature-name)az appconfig enhanced-feature-flag enable(required:--feature-name)az appconfig enhanced-feature-flag disable(required:--feature-name)Highlights:
.appconfig.featureflag/--keyproperty is intentionally not exposed (the dedicated/ff/endpoint manages the key server-side).--telemetry-enabled,--requirement-type,--description, and--tagsonset.setalso accepts--flag, which replaces the entire feature flag using the same property names, casing and types as the data-plane/ffAPI andaz appconfig enhanced-feature-flag show(name,enabled,description,conditions,allocation,variants,telemetry,tags).--flagaccepts a JSON object, an AAZ shorthand-syntax object, a file path with@prefix (e.g.@flag.json), or@-to read from stdin. Because it is a full-object replacement,--flagcannot be combined with--enable,--description,--requirement-typeor--telemetry-enabled.--fieldssupportsname,enabled,label,description,conditions,variants,allocation,tags,last_modified.setperforms a get-then-update so unspecified fields (e.g.enabled,description,telemetry,tags) are preserved on update.Examples
Set an enhanced feature flag from a full flag object using shorthand syntax (no JSON quoting needed; values with spaces/commas are wrapped in single quotes):
The shorthand object is equivalent to this JSON (also accepted via
--flag '<json>',--flag @flag.json, or--flag @-):{"name":"Beta","enabled":true,"description":"Set via --flag","conditions":{"requirement_type":"All","filters":[{"name":"Microsoft.TimeWindow","parameters":{"Start":"Wed, 01 Jan 2025 00:00:00 GMT"}}]}}Testing
test_azconfig_enhanced_featurescenario test (with recording) covering create/update/show/list/enable/disable/delete plus not-found error paths, including setting a flag via--flag(both raw JSON and shorthand syntax), the--flag/content-argument mutual-exclusivity checks, and the--flag/--feature-namename-mismatch validation.This checklist is used to make sure that common guidelines for a pull request are followed.
HISTORY.rstentry.