Adding the az managementgroup extension for Management Groups #4198
Conversation
ghost
added
the
customer-reported
|
Thank you for your contribution git-thomasdolan! We will review the pull request and get back to you soon. |
|
|
…-thomasdolan/azure-cli-extensions into thomasdolan/old_swagger_test
git-thomasdolan
changed the title
|
managementgroup |
.github/CODEOWNERS
Outdated
|
|
||
| /src/init/ @zhoxing-ms @HuangYT2000 | ||
|
|
||
| /src/managementgroup/ @kairu-ms |
There was a problem hiding this comment.
@git-thomasdolan Just out of curiosity, is this extension the owned by your team or Azure CLI team?
If your team is responsible for it's development and maintenance, the owner should be a member from your team
There was a problem hiding this comment.
Since I generated the commands, I can put myself as an owner
| ##### Show ##### | ||
| ``` | ||
| az managementgroup management-group show --group-id "20000000-0001-0000-0000-000000000000" | ||
| ``` | ||
| ##### Show ##### | ||
| ``` | ||
| az managementgroup management-group show --expand "ancestors" --group-id "20000000-0001-0000-0000-00000000000" | ||
| ``` | ||
| ##### Show ##### | ||
| ``` | ||
| az managementgroup management-group show --expand "children" --group-id "20000000-0001-0000-0000-000000000000" | ||
| ``` | ||
| ##### Show ##### | ||
| ``` | ||
| az managementgroup management-group show --expand "path" --group-id "20000000-0001-0000-0000-000000000000" | ||
| ``` | ||
| ##### Show ##### | ||
| ``` | ||
| az managementgroup management-group show --expand "children" --recurse true \ | ||
| --group-id "20000000-0001-0000-0000-000000000000" | ||
| ``` |
There was a problem hiding this comment.
Show
Could we have a more detailed description of this title, or could we put similar contents together under one title
| 'subscriptions from results (i.e. \'$filter=children.childType ne Subscription\')') | ||
|
|
||
| with self.argument_context('managementgroup management-group-subscription create') as c: | ||
| c.argument('group_id', type=str, help='Management Group ID.') |
There was a problem hiding this comment.
type=str is the default setting, it does not need to be explicitly declared
| c.argument('skiptoken', type=str, help='Page continuation token is only used if a previous operation returned ' | ||
| 'a partial result. If a previous response contains a nextLink element, the value of the nextLink ' | ||
| 'element will include a token parameter that specifies a starting point to use for subsequent ' | ||
| 'calls.') |
| c.argument('skiptoken', type=str, help='Page continuation token is only used if a previous operation returned ' | ||
| 'a partial result. If a previous response contains a nextLink element, the value of the nextLink ' | ||
| 'element will include a token parameter that specifies a starting point to use for subsequent ' | ||
| 'calls.') |
| c.argument('select', type=str, help='This parameter specifies the fields to include in the response. Can ' | ||
| 'include any combination of Name,DisplayName,Type,ParentDisplayNameChain,ParentChain, e.g. ' | ||
| '\'$select=Name,DisplayName,Type,ParentDisplayNameChain,ParentNameChain\'. When specified the ' | ||
| '$select parameter can override select in $skipToken.') | ||
| c.argument('search', arg_type=get_enum_type(['AllowedParents', 'AllowedChildren', | ||
| 'ParentAndFirstLevelChildren', 'ParentOnly', 'ChildrenOnly']), | ||
| help='The $search parameter is used in conjunction with the $filter parameter to return three ' | ||
| 'different outputs depending on the parameter passed in. With $search=AllowedParents the API will ' | ||
| 'return the entity info of all groups that the requested entity will be able to reparent to as ' | ||
| 'determined by the user\'s permissions. With $search=AllowedChildren the API will return the entity ' | ||
| 'info of all entities that can be added as children of the requested entity. With ' | ||
| '$search=ParentAndFirstLevelChildren the API will return the parent and first level of children ' | ||
| 'that the user has either direct access to or indirect access via one of their descendants. With ' | ||
| '$search=ParentOnly the API will return only the group if the user has access to at least one of ' | ||
| 'the descendants of the group. With $search=ChildrenOnly the API will return only the first level ' | ||
| 'of children of the group entity info specified in $filter. The user must have direct access to ' | ||
| 'the children entities or one of it\'s descendants for it to show up in the results.') | ||
| c.argument('filter_', options_list=['--filter'], type=str, help='The filter parameter allows you to filter on ' | ||
| 'the the name or display name fields. You can check for equality on the name field (e.g. name eq ' | ||
| '\'{entityName}\') and you can check for substrings on either the name or display name fields(e.g. ' | ||
| 'contains(name, \'{substringToSearch}\'), contains(displayName, \'{substringToSearch\')). Note that ' | ||
| 'the \'{entityName}\' and \'{substringToSearch}\' fields are checked case insensitively.') |
There was a problem hiding this comment.
Please refine the help message, because It seems to illustrate the feature of this parameter from the perspective of REST API rather than from the perspective of command parameters.
Please explain the parameters from the perspective of command parameters, otherwise users will be confused.
| c.argument('group_name', type=str, help='A filter which allows the get entities call to focus on a particular ' | ||
| 'group (i.e. "$filter=name eq \'groupName\'")') |
There was a problem hiding this comment.
Please refine the help message, because It seems to illustrate the feature of this parameter from the perspective of REST API rather than from the perspective of command parameters.
Please explain the parameters from the perspective of command parameters, otherwise users will be confused.
| g.custom_command('show-subscription', 'managementgroup_management_group_subscription_show_subscription') | ||
| g.custom_command( | ||
| 'show-subscription-under-management-group', | ||
| 'managementgroup_management_group_subscription_show_subscription_under_management_group', | ||
| ) |
There was a problem hiding this comment.
The command signature show-subscription-under-management-group seems to be too long and inconvenient for users to input
Could we distinguish these two scenarios by parameters rather than by command signature?
| step_entity_list(test, checks=[]) | ||
| #step_hierarchy_setting_create(test, checks=[]) | ||
| #step_hierarchy_setting_show(test, checks=[]) | ||
| #step_hierarchy_setting_list(test, checks=[]) | ||
| #step_hierarchy_setting_update(test, checks=[]) | ||
| #step_hierarchy_setting_delete(test, checks=[]) | ||
| step_start_tenant_backfill(test, checks=[]) | ||
| step_tenant_backfill_status(test, checks=[]) | ||
| step_management_group_create(test, checks=[]) | ||
| step_management_group_show_descendant(test, checks=[]) | ||
| step_management_group_show(test, checks=[]) | ||
| step_management_group_show2(test, checks=[]) | ||
| step_management_group_show3(test, checks=[]) | ||
| step_management_group_show4(test, checks=[]) | ||
| step_management_group_show5(test, checks=[]) | ||
| step_management_group_list(test, checks=[]) | ||
| step_management_group_update(test, checks=[]) | ||
| step_management_group_delete(test, checks=[]) | ||
| #step_management_group_subscription_create(test, checks=[]) | ||
| #step_management_group_subscription(test, checks=[]) | ||
| #step_management_group_subscription2(test, checks=[]) | ||
| #step_management_group_subscription_delete(test, checks=[]) |
There was a problem hiding this comment.
Please add checks for those tests
| "Command": "az azurestackhci", | ||
| "AzureServiceName": "Azure Stack HCI", | ||
| "URL": "https://docs.microsoft.com/en-us/cli/azure/azurestackhci?view=azure-cli-latest" | ||
| "URL": "https://docs.microsoft.com/en-us/cli/azure/stack-hci?view=azure-cli-latest" |
There was a problem hiding this comment.
May I ask why do you need to modify this URL?
|
I see that some commands with the same features have been included in the official CLI, could you confirm it? code link |
Yes, the commands have the same features in this new extension as they do in the link you sent. However, that has not been updated in a long time. I believe the 'az account managementgroup' is a 2018 API. There have been changes to those commands and new commands have been added to our API. So the 'az account managementgroup' also needs to be updated or it should be replaced by this new extension. What would you recommend? |
Co-authored-by: Xing Zhou <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
|
@git-thomasdolan I suggest updating those commands in CLI main repo, including upgrading the api-version for Because the commands with same features in CLI should not have two different interactive interfaces, otherwise it will increase the user's additional learning cost and confuse users. |
|
@zhoxing-ms I started looking into what you mentioned yesterday and it seems like the python sdk for management groups was updated to our latest version here. If that's the case, why would the current managementgroup cli main module look at an older version? |
|
@git-thomasdolan This is because the Python SDK version used in CLI is still the older version. code link |
|
@zhoxing-ms I Created a draft PR here. Found that the latest version of the azure-mgt-managementgroup python sdk is 1.0.0. Do I need to update the tests for managementgroup? If so how do I test? I have only done tests for the cli extensions so I am unfamiliar with this process. |
|
@git-thomasdolan Hi, I have commented here Azure/azure-cli#20942 (comment) |
|
@git-thomasdolan I'd like ask whether the contents in this PR are included in #20942? If we don't need this PR any more, please close it |
|
This PR is not needed anymore, it is fully covered in #20942 |
3 participants
closed Azure/azure-cli#20265
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally?For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update
src/index.jsonautomatically.The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify
src/index.json.