Add management groups commands to azure-cli-resource #6426

rajshah11 · 2018-05-24T10:51:24Z

This checklist is used to make sure that common guidelines for a pull request are followed.

The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).
I adhere to the Command Guidelines.

promptws · 2018-05-24T10:51:40Z

View a preview at https://prompt.ws/r/Azure/azure-cli/6426
This is an experimental preview for @microsoft users.

rajshah11 · 2018-05-24T10:54:51Z

The SDK is in PR Azure/azure-sdk-for-python#2520. I have tested by downloading the wheel into the "privates" folder. The SDK will be merged and release once this PR is given a thumbs up. Adding @lmazuel

tjprescott

To transition from an extension to being in the CLI, a command review is required. Please reach out to schedule.

tjprescott · 2018-05-24T16:39:04Z

src/command_modules/azure-cli-resource/HISTORY.rst

@@ -3,6 +3,10 @@
 Release History
 ===============

+2.0.30
++++++
+* Add management-group commands


This needs to be more explicit especially since these commands live in the command tree that is predominantly in a different module.
"* add account management-group commands."

Ok will make this change. Also, just checked the travis build. Will make the changes.

lmazuel · 2018-05-25T16:26:22Z

src/command_modules/azure-cli-resource/setup.py

@@ -33,7 +33,8 @@
 DEPENDENCIES = [
    'azure-mgmt-resource==1.2.1',
    'azure-cli-core',
-    'azure-mgmt-authorization==0.40.0'
+    'azure-mgmt-authorization==0.40.0',
+    'azure-mgmt-managementgroups==2018-03-01-preview'


Just to be sure we're on the same page 2018-03-01-preview is a temporary version picked by the SDK CI, actual final version will be 0.1.0.
So once we're done here, final step will be:

I release the SDK as 0.1.0 on PyPI

You do a last commit here that removes the "privates" folder, and change this line to 0.1.0

Yes. That's right.

tjprescott

Summary of changes we discussed during the review:

all examples need to be updated
remove --group-name and just use --name
--parent-id should be just --parent and accept the name or ID of another management group

tjprescott

Some feedback on certain implementation details. Didn't see anything obvious that would be not working about parent.

tjprescott · 2018-05-30T17:07:30Z

src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/custom.py

@@ -1077,8 +1077,119 @@ def update_policy_setdefinition(cmd, policy_set_definition_name, definitions=Non
    return policy_client.policy_set_definitions.create_or_update(policy_set_definition_name, parameters)


+def _register_rp(cli_ctx, subscription_id=None):


Why is this in here? @yugangw-msft @lmazuel correct me if I'm wrong but msrest will automatically handle this, correct?

You can look at the discussion here - Azure/azure-cli-extensions#62. Not sure if the issue has been resolved.

tjprescott · 2018-05-30T17:11:17Z

src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/custom.py

+
+def cli_managementgroups_group_update_get():
+    update_parameters = {'display_name': None, 'parent': None}
+    return update_parameters


Instead of creating a dictionary, you could just create the PatchManagementGroupRequest object here.

Will do. Thanks!

tjprescott · 2018-05-30T17:12:05Z

src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/custom.py

+        parent=None):
+    instance["display_name"] = display_name
+    instance["parent"] = parent
+    return instance


If the instance is the object, these simply become instance.foo = foo. A little cleaner.

tjprescott · 2018-05-30T17:13:16Z

src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/custom.py

+    parent_id=_get_parent_id_from_parent(parameters["parent"])
+    from azure.mgmt.managementgroups.models import PatchManagementGroupRequest
+    patch_mgmt_grp_request = PatchManagementGroupRequest(display_name=parameters["display_name"], parent_id=parent_id)
+    return client.update(group_name, patch_mgmt_grp_request)


This could be made cleaner:

def cli_managmentgroups_group_update_set(…): return client.update(group_name, parameters)

troydai · 2018-05-30T21:03:10Z

@rajshah11 the sprint 38 will close by the end of tomorrow. Please make sure this PR is merged by then.

rajshah11 · 2018-05-31T04:28:00Z

I have made all the suggested changes and fixed some of the build errors. However, there are still a few errors. Can someone please look into this build and assist me in removing those errors - https://travis-ci.org/Azure/azure-cli/builds/385959671?utm_source=github_status&utm_medium=notification?

Firstly, I am getting an error saying
FAIL: unrecognized_help_parameter_rule
Help-Entry: account management-group subscription add - The following parameter help names are invalid: --subscription
Help-Entry: account management-group subscription remove - The following parameter help names are invalid: --subscription
However, account management-group subscription add -h does show the parameter and command works as well.

Also, the automation tests are failing but the tests run properly on my machine.

…t-grp

tjprescott

Two things:

Add logic to suppress the old extension. https://github.com/Azure/azure-cli/blob/dev/doc/authoring_command_modules/authoring_commands.md#extension-suppression
Remove private WHL

codecov-io · 2018-05-31T20:26:22Z

Codecov Report

Merging #6426 into dev will not change coverage.
The diff coverage is n/a.

@@         Coverage Diff         @@
##           dev   #6426   +/-   ##
===================================
  Coverage    0%      0%           
===================================
  Files       11      11           
  Lines      133     133           
  Branches     9       9           
===================================
  Misses     133     133

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2cd0ed3...e73a0a6. Read the comment docs.

tjprescott · 2018-05-31T20:56:33Z

Also please make sure you are running azdev style before pushing your changes. That way you'll find out quickly if there are pylint/pep8 errors instead of waiting for the CI to tell you.

jiasli · 2023-11-14T02:50:24Z

src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/_help.py

+helps['account management-group create'] = """
+    type: command
+    short-summary: Create a new management group.
+    long-summary: Create a new management group.


long-summary should provide additional information, instead of being a duplicate of short-summary.

rajshah11 added 2 commits May 24, 2018 03:32

Add mgmt groups to resource, use latest SDK, update tests

ae58bff

Bump up version and edit history

42376be

Add wheel file

89210ef

style check

07e1bf3

tjprescott suggested changes May 24, 2018

View reviewed changes

tjprescott added the New Service label May 24, 2018

lmazuel reviewed May 25, 2018

View reviewed changes

tjprescott suggested changes May 29, 2018

View reviewed changes

Changes suggested in PR/after review

80d08ab

tjprescott added this to the Sprint 38 milestone May 30, 2018

tjprescott reviewed May 30, 2018

View reviewed changes

rajshah11 added 3 commits May 30, 2018 15:56

More changes suggested in the PR and rerun tests

972a0e6

Style check

6d06867

Some build fixes

25e6137

rajshah11 force-pushed the cli-mgmt-grp branch 3 times, most recently from ebee1a0 to 385ff5d Compare May 31, 2018 00:17

Minor fix

9a9566c

rajshah11 force-pushed the cli-mgmt-grp branch from 385ff5d to 9a9566c Compare May 31, 2018 00:20

tjprescott mentioned this pull request May 31, 2018

Allow one "bubble" in the perf script. #6470

Merged

rajshah11 force-pushed the cli-mgmt-grp branch from 0450fca to 0a52fbe Compare May 31, 2018 19:52

Fix build errors, rerun tests in prod, update help

21ddb6b

rajshah11 force-pushed the cli-mgmt-grp branch from 0a52fbe to 21ddb6b Compare May 31, 2018 19:56

Merge branch 'dev' of https://github.com/Azure/azure-cli into cli-mgm…

4266a02

…t-grp

tjprescott suggested changes May 31, 2018

View reviewed changes

Supress extension

698c379

rajshah11 force-pushed the cli-mgmt-grp branch from 21f1ca9 to 698c379 Compare May 31, 2018 20:38

Remove private whl file

e73a0a6

rajshah11 force-pushed the cli-mgmt-grp branch from b7a3c4e to e73a0a6 Compare May 31, 2018 21:19

tjprescott approved these changes May 31, 2018

View reviewed changes

tjprescott merged commit 6ffea89 into Azure:dev May 31, 2018

haroldrandom added the New Service label Oct 25, 2019

jiasli mentioned this pull request Oct 14, 2021

Unable to create child management group as of version 2.27.1 #19511

Closed

jiasli reviewed Nov 14, 2023

View reviewed changes

This was referenced Nov 14, 2023

[ARM] Bump version of azure-mgmt-managementgroups from 0.2.0 to 1.0.0 (Track 2 SDK) and add new sub commands for management-group #20942

Merged

{ARM} Remove duplicate help messages #27836

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add management groups commands to azure-cli-resource #6426

Add management groups commands to azure-cli-resource #6426

rajshah11 commented May 24, 2018

promptws commented May 24, 2018

rajshah11 commented May 24, 2018

tjprescott left a comment

tjprescott May 24, 2018

rajshah11 May 24, 2018 •

edited

Loading

lmazuel May 25, 2018

rajshah11 May 29, 2018

tjprescott left a comment

tjprescott left a comment

tjprescott May 30, 2018

rajshah11 May 30, 2018 •

edited

Loading

tjprescott May 30, 2018

rajshah11 May 30, 2018

tjprescott May 30, 2018

tjprescott May 30, 2018

troydai commented May 30, 2018 •

edited

Loading

rajshah11 commented May 31, 2018 •

edited

Loading

tjprescott left a comment

codecov-io commented May 31, 2018 •

edited

Loading

tjprescott commented May 31, 2018

jiasli Nov 14, 2023

		@@ -1077,8 +1077,119 @@ def update_policy_setdefinition(cmd, policy_set_definition_name, definitions=Non
		return policy_client.policy_set_definitions.create_or_update(policy_set_definition_name, parameters)


		def _register_rp(cli_ctx, subscription_id=None):

Add management groups commands to azure-cli-resource #6426

Add management groups commands to azure-cli-resource #6426

Conversation

rajshah11 commented May 24, 2018

promptws commented May 24, 2018

rajshah11 commented May 24, 2018

tjprescott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rajshah11 May 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjprescott left a comment

Choose a reason for hiding this comment

tjprescott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rajshah11 May 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

troydai commented May 30, 2018 • edited Loading

rajshah11 commented May 31, 2018 • edited Loading

tjprescott left a comment

Choose a reason for hiding this comment

codecov-io commented May 31, 2018 • edited Loading

Codecov Report

tjprescott commented May 31, 2018

Choose a reason for hiding this comment

rajshah11 May 24, 2018 •

edited

Loading

rajshah11 May 30, 2018 •

edited

Loading

troydai commented May 30, 2018 •

edited

Loading

rajshah11 commented May 31, 2018 •

edited

Loading

codecov-io commented May 31, 2018 •

edited

Loading