{Network} Connection monitor V2 preview by haroldrandom · Pull Request #1238 · Azure/azure-cli-extensions

haroldrandom · 2020-02-02T16:00:13Z

Temporary hotfix Azure/azure-cli#9432

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

General Guidelines

Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

My extension description/summary conforms to the Extension Summary Guidelines.

…uments

…er set CM

azuresdkci · 2020-02-02T16:00:35Z

If this PR is for a new extension or change to an existing extension, use the following to try out the changes in this PR:

docker run -it microsoft/azure-cli:latest
export EXT=<NAME>
pip install --upgrade --target ~/.azure/cliextensions/$EXT "git+https://github.com/haroldrandom/azure-cli-extensions.git@connection-monitor-v2#subdirectory=src/$EXT&egg=$EXT"

mmyyrroonn

Good job! LGTM in general. Could we move one more test from CLI core in here. There is a test called test_network_watcher_connection_monitor in main CLI. I saw several tests about V2 cm which is awesome. Better to test some cross situations so that we won't bring breaking change for current users.

src/connection-monitor-preview/README.md

src/connection-monitor-preview/azext_connection_monitor_preview/commands.py

mmyyrroonn · 2020-02-03T02:49:28Z

src/connection-monitor-preview/azext_connection_monitor_preview/custom.py

+                                 connection_monitor_name,
+                                 watcher_rg,
+                                 watcher_name,
+                                 resource_group_name=None,


want to confirm. Is resource_group_name mandatory?

No, This is a weird parameter...Even though it was set in function signature through process_nw_cm_create_namespace(cmd, namesapce) in _validators.py, but eventually it's useless, it uses watcher_rg finally in create_nw_connection_monitor.

So I am also get confused by the old code.

src/connection-monitor-preview/azext_connection_monitor_preview/custom.py

src/connection-monitor-preview/azext_connection_monitor_preview/_params.py

src/connection-monitor-preview/azext_connection_monitor_preview/commands.py

mmyyrroonn · 2020-02-03T03:28:40Z

src/connection-monitor-preview/azext_connection_monitor_preview/_params.py

+            c.extra('location', get_location_type(self.cli_ctx), required=True)
+            c.argument('resource_group_name', arg_type=ignore_type, validator=nw_validator)
+
+    with self.argument_context('network watcher connection-monitor create', arg_group='V1') as c:


should it be arg_group='V1 Endpoint'?

I re-group them,
V1 Endpoint Arguemnt incldues: --source-xxxx, --dest-xxx, --do-not-start, --monitoring-interval
V2 Argument includes only: --notes

I re-group them:

mmyyrroonn · 2020-02-03T03:29:05Z

src/connection-monitor-preview/azext_connection_monitor_preview/_params.py

+                   help='Create the connection monitor but do not start it immediately.')
+        # c.ignore('location')
+
+    with self.argument_context('network watcher connection-monitor', min_api='2019-11-01') as c:


Is this used both in V1 and V2?

No. It is used to create V2 only. What do you suggest?

arg group maybe?

I re-group them,
V1 Endpoint Arguemnt incldues: --source-xxxx, --dest-xxx, --do-not-start, --monitoring-interval
V2 Argument includes only: --notes

I re-group them:

mmyyrroonn · 2020-02-03T03:33:45Z

src/connection-monitor-preview/azext_connection_monitor_preview/commands.py

+                            nw_connection_monitor_sdk,
+                            client_factory=cf_nw_connection_monitor,
+                            min_api='2018-01-01') as g:
+        g.custom_command('create', 'create_nw_connection_monitor', validator=process_nw_cm_create_namespace)


could we add more help information in _help.py for this command? like what we need to create a V2 cm. I mean in long summary part.

Could you be more specifically?

Long summary here:

mmyyrroonn

Other part is ok. If we meet more issues, we can fix them in our main CLI

mmyyrroonn · 2020-02-04T03:28:09Z

...tor-preview/azext_connection_monitor_preview/tests/latest/test_connection_monitor_preview.py

+                             src_resource_name=source_name,
+                             src_resource_id=source_resource_id)
+
+        # connection monitor V2 is in preview stage


Hello. Is this by purpose. should it success?

As talked at Teams, believe it could be some transient issue of conflict error with ARM/LA.

Change default location to eastus in test

yonzhan · 2020-02-04T04:16:36Z

add to S165.

Jianhui Harold added 30 commits January 16, 2020 10:31

Init SDK of API Version 2019-11-01

d26914d

Init HISTORY.rst

71a48e8

Init README.md

24d14cf

Init setup.py

d51c979

Init extension structure

3ff1ca4

Init draft code

80d7ef6

Test get connection monitor and argparse action

5ef270c

Overwrite az network watcher connection-monitor create

4435032

Init _create_nw_connection_monidor_endpoint() and refect endpoint arg…

e7e0640

…uments

Add parameters for test-configuration and test-group when creating CMv2

3d6eefb

Init _create_nw_connection_monitor_v2_test_configuration()

1707ef3

Init _create_nw_connection_monitor_v2_test_group()

bf2e375

Add default value to accelerate creation

4abb624

Init command network watcher connection-monitor endpoint add

ce50a54

Init command network watcher connection-monitor endpoint remove

e4e7449

Init command network watcher connection-monitor endpoint show

57ca778

Init command network watcher connection-monitor endpoint list

c5498c5

command: network watcher connection-monitor test-configuration add

3382cc5

command: network watcher connection-monitor test-configuration remove

04c2892

command: network watcher connection-monitor test-configuration show

412e930

command: network watcher connection-monitor test-configuration list

eca71a2

command: network watcher connection-monitor test-group add

c45f5ac

command: network watcher connection-monitor test-group remove

c19c939

command: network watcher connection-monitor test-group show

c96f555

command: network watcher connection-monitor test-group list

cbaf956

Refactor SDK directory

df0c0e5

Refactor SDK directory

356e720

Add SDK of API Version 2019-06-01

25f05a1

Hack _create_nw_connection_monitor_v1() to allow to create V1 paramet…

4f79f12

…er set CM

Add test test_nw_connection_monitor_v2_creation

924767d

Jianhui Harold added 5 commits February 2, 2020 17:48

Add test_connection_monitor_v2_test_group

6d1d5a8

Add output option for create a V2 connection monitor

5254a93

command: network watcher connection-monitor output add

cf52a61

command: network watcher connection-monitor output remove

5d87d49

command: network watcher connection-monitor output list

cf561f2

haroldrandom self-assigned this Feb 2, 2020

Jianhui Harold added 4 commits February 3, 2020 00:04

Merge branch 'master' into connection-monitor-v2

104e3c6

Fix code style error

e6914d3

Fix flake8 error

eebac19

Update github code owner

ad43806

haroldrandom marked this pull request as ready for review February 2, 2020 16:55

haroldrandom requested a review from mmyyrroonn February 2, 2020 16:55

mmyyrroonn reviewed Feb 3, 2020

View reviewed changes

Jianhui Harold added 5 commits February 3, 2020 16:45

Use custom_show_command() instead custom_command()

05726c7

Add azure-cli>=2.0.80 requirement warning

113f1d8

Add test_nw_connection_monitor_v1

c7e234d

Add long summary of help command and regroup arguments

81d03fa

Merge branch 'master' into connection-monitor-v2

074a669

mmyyrroonn reviewed Feb 4, 2020

View reviewed changes

yonzhan added this to the S165 milestone Feb 4, 2020

yonzhan requested a review from jsntcy February 4, 2020 04:16

Change default location to westus in test

1d50147

mmyyrroonn approved these changes Feb 4, 2020

View reviewed changes

haroldrandom merged commit 0284ef6 into Azure:master Feb 4, 2020

haroldrandom deleted the connection-monitor-v2 branch February 4, 2020 07:34

haroldrandom mentioned this pull request Feb 4, 2020

{Network} Update index.json for connection-monitor-preview #1242

Merged

3 tasks

ManuInNZ pushed a commit to ManuInNZ/azure-cli-extensions that referenced this pull request Apr 11, 2020

{Network} Connection monitor V2 preview (Azure#1238)

9247e40

Conversation

haroldrandom commented Feb 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

General Guidelines

Uh oh!

azuresdkci commented Feb 2, 2020

Uh oh!

mmyyrroonn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmyyrroonn left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yonzhan commented Feb 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

haroldrandom commented Feb 2, 2020 •

edited

Loading

mmyyrroonn left a comment •

edited

Loading