[EventGrid] Add CLI support for 2020-04-01-preview and mark preview features with is_Preview=True by ahamad-MS · Pull Request #14027 · Azure/azure-cli

ahamad-MS · 2020-06-18T23:12:31Z

Description

This PR is to add support for all the previewed and GA'ed features from event Grid CLI extension. We are marking all the features that are still in preview with the proper is_Preview tag and will be using this mechanism with all future releases.

Features included in GA:
- Event Delivery Schema
- Input Mapping.
- Custom Input Schema EventDelivery Schema
- Cloud Event V10 Schema
- Service Bus Topic As Destination
- Azure Function As Destination
- WebHook Batching
- Secure webhook (AAD support)
- ImmutableId
- IpFiltering
- Private Link
Features that are still in preview:
- Partner Topic
- Msi For Delivery To First Party Destinations
- Sku
- Tracked System Topic

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.

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

The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.

…ons (Azure#12771)

…review=true for previewed features only and re-record the tests

yonzhan · 2020-06-19T00:55:00Z

Event Grid

ahamad-MS · 2020-06-19T23:08:33Z

Hello @myronfanqiu @haroldrandom , @qianwens and @Juliehzl , Can you please helpo review this PR and help with failures. Some of them does not make sense as these same arguments/commands were used in CLI extension. For example, one of the linter errors complains that the argument destination-resource-group ends with resource-group in the name. I changed that to destination-resource-group-name and the same error occurs.

Can you please provide some help here?

thanks

mmyyrroonn · 2020-06-22T01:39:25Z

/azp run

azure-pipelines · 2020-06-22T01:39:39Z

Azure Pipelines successfully started running 2 pipeline(s).

mmyyrroonn · 2020-06-22T01:41:30Z

@ahamad-MS Hi, please solve other CI issues except for the linter one. We're discussing about the resource group naming rule.

haroldrandom · 2020-06-22T01:41:56Z

Hi, @ahamad-MS .

I don't see what you said related to the CI failure. Instead,

  File "/opt/hostedtoolcache/Python/3.6.10/x64/lib/python3.6/site-packages/azure/cli/command_modules/eventgrid/custom.py", line 14, in <module>
    from azure.mgmt.eventgrid.models import (
ImportError: cannot import name 'ResourceSku'

ahamad-MS · 2020-06-22T06:42:37Z

Hello @haroldrandom .. This error is observed locally when I ran azdev liter eventgrid.

you are right that import error comes in the online check and I am not sure why this is also happening. i dont see it locally. Any suggestion on how to resolve it?

haroldrandom · 2020-06-22T09:46:02Z

Hello @haroldrandom .. This error is observed locally when I ran azdev liter eventgrid.

you are right that import error comes in the online check and I am not sure why this is also happening.

The output indicates the file azure/mgmt/eventgrid/models/__init__.py doesn't export ResourceSku. I think the problem hides in SDK generation. You could try to import it manually in Python REPL to test whether it's importable or not. And then, add it explictily in __init__.py to export and import again.

The question why it doesn't occur previously is that the class ResourceSku never used in custom.py

ahamad-MS · 2020-06-23T03:56:37Z

Thanks @haroldrandom for the discussion on teams. As I showed you, the python SDK module for EventGrid is correctly installed on my local machine and my local tests are running fine with no issues along with azdev style.

When looking in model files under $root\azure-cli\env3\Lib\site-packages\azure\mgmt\eventgrid\models , we saw resourcesku was defined and used correctly. Also, init.py file was referencing/importing it correctly.

Please note the setup.py file is referring the correct python package (namely, 'azure-mgmt-eventgrid~=3.0.0rc7',)

Hence, I believe it is something related to the github setting or the check itself where it is not correctly referencing the correct package. This seems an infra issue

Please let me know if more info needed.

ahamad-MS · 2020-06-29T21:51:46Z

@myronfanqiu All comments addressed. If additional info needed, let us discuss in person.

ahamad-MS · 2020-06-30T07:46:48Z

@yonzhan Can you please provide an ETA when this PR will be merged? thanks

yonzhan · 2020-06-30T11:04:11Z

@ahamad-MS @Juliehzl will help you merge it by this Sprint.

Juliehzl · 2020-07-01T07:50:37Z

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

+  - name: List all partner registrations in a resource group whose name contains the pattern "XYZ"
+    text: az eventgrid partner registration list -g rg1 --odata-query "Contains(name, 'XYZ')"
+  - name: List all partner registrations in a resource group except the partner registration with name "name1"
+    text: az eventgrid partner registration list -g rg1 --odata-query "NOT (name eq 'name1')"


should --odata-query be --filter? cli already has a global parameter named --query.

No. This is how we handle pagination and search filtering for all existing commands

Juliehzl · 2020-07-01T07:52:19Z

src/azure-cli/azure/cli/command_modules/eventgrid/_params.py


 included_event_types_type = CLIArgumentType(
-    help="A space-separated list of event types (e.g., Microsoft.Storage.BlobCreated Microsoft.Storage.BlobDeleted). To subscribe to all default event types, do not specify this argument. For event grid topics, event types are customer defined. For Azure events, e.g., Storage Accounts, IoT Hub, etc., you can query their event types using this CLI command 'az eventgrid topic-type list-event-types'.",
+    help="A space-separated list of event types (e.g., Microsoft.Storage.BlobCreated and Microsoft.Storage.BlobDeleted). In order to subscribe to all default event types, do not specify any value for this argument. For event grid topics, event types are customer defined. For Azure events, e.g., Storage Accounts, IoT Hub, etc., you can query their event types using this CLI command 'az eventgrid topic-type list-event-types'.",


Is there a way to get all related types?

The "provided" help text is already helping user to get this info specifically:

you can query their event types using this CLI command 'az eventgrid topic-type list-event-types'.

src/azure-cli/azure/cli/command_modules/eventgrid/_params.py

Juliehzl · 2020-07-01T07:59:38Z

src/azure-cli/azure/cli/command_modules/eventgrid/_params.py

        c.argument('domain_name', arg_type=domain_name_type)
        c.argument('domain_topic_name', arg_type=domain_topic_name_type)
+        c.argument('system_topic_name', arg_type=system_topic_name_type)
+        c.argument('source', help="The ARM Id for the topic, e.g., /subscriptions/{SubId}/resourceGroups/{RgName}/providers/Microsoft.Storage/storageAccounts/{AccountName}")


--source should be arm resource id for topic, right? could we use topic resource id as example not storage account resource id?

No. This is not same as resource id. the word "source" needs to be maintained and used here. Also, this copied exactly from Event Grid CLI extension and user are using it this way.

From the example you provided, it is storage account resource id, right? CLI extension is used for private preview commands and for cli modules, we need to be consistent with cli command guidline.
here we want the description for --source could be more clear. If it is not resource id, could you provide another example?

@Juliehzl source is a concept used in Event Grid and messaging world. it just happens to be the arm id for the system topic (Storage in this case). We will not change the name (source) as this is the expected name we want to use in our documentation and command. this is unique to event grid and we use source-resource-id in event subsription creation/update command and yes it matches what you are talking about. The case here is totally different.

This is still by design and won't fix.

Juliehzl · 2020-07-01T08:06:13Z

src/azure-cli/azure/cli/command_modules/eventgrid/commands.py

+
+    with self.command_group('eventgrid system-topic', system_topics_mgmt_util, client_factory=system_topics_factory, is_preview=True) as g:
+        g.show_command('show', 'get')
+        g.command('delete', 'delete')


could we add confirmation=True for all delete commands?

No. We keep consistency with all other existing delete commands.

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

ahamad-MS · 2020-07-01T23:14:11Z

Hello @Juliehzl all comments resolved. Please merge the changes asap. thx

mmyyrroonn

Approved for private link part. 😃

yonzhan · 2020-07-03T01:01:04Z

@Juliehzl Please help review this PR and get it merged by this Sprint.

Juliehzl · 2020-07-03T02:39:48Z

Hello @Juliehzl all comments resolved. Please merge the changes asap. thx

#14027 (comment)

ahamad-MS · 2020-07-03T15:40:35Z

@Juliehzl ... #14027 (comment) is already addressed and it is Won't Fix. Source is not Azure concept here. It is Event Grid concept stems from Cloud Event 1.0 specification. The world "source" is what we want to use and what makes sense to our customer. it can be Azure ARM ID or external to azure.

ahamad-MS · 2020-07-05T23:24:18Z

@Juliehzl What is the ETA for merging this change?

src/azure-cli/azure/cli/command_modules/eventgrid/_client_factory.py

src/azure-cli/azure/cli/command_modules/eventgrid/_params.py

ahamad-MS · 2020-07-06T09:09:09Z

@myronfanqiu All new comments addressed on 7/6 are addressed.

dkmiller and others added 8 commits April 27, 2020 16:13

{Docs} Remove stale reference in README to closed issue about extensi…

00b9235

…ons (Azure#12771)

Merge remote-tracking branch 'upstream/release'

ab16df0

Merge remote-tracking branch 'upstream/release'

03436bc

Merge remote-tracking branch 'upstream/release'

7d98b1f

Merge remote-tracking branch 'upstream/release'

06f6ec5

Merge remote-tracking branch 'upstream/master' into dev

7313ae5

Merge remote-tracking branch 'upstream/dev' into dev

aea3707

Add CLI support for Azure Event Grid 2020-04-01-preview and mark is_p…

10c3c87

…review=true for previewed features only and re-record the tests

ahamad-MS requested review from Juliehzl and haroldrandom as code owners June 18, 2020 23:12

yonzhan assigned Juliehzl Jun 19, 2020

yonzhan requested a review from qianwens June 19, 2020 00:54

yonzhan added this to the S172 milestone Jun 19, 2020

ahamad-MS and others added 6 commits June 18, 2020 20:56

Merge branch 'dev' into dev

16fbe29

remove secret

9837f31

Merge branch 'dev' of https://github.com/ahamad-MS/azure-cli into dev

409e43f

Merge remote-tracking branch 'upstream/dev' into dev

fbcf7e1

remove secrets from tests

9f28ecd

fix linter

096fb10

yonzhan requested a review from mmyyrroonn June 19, 2020 23:12

mmyyrroonn mentioned this pull request Jun 22, 2020

Downgrade parameter_should_not_end_in_resource_group's severity to Medium Azure/azure-cli-dev-tools#216

Merged

Ashraf Hamad added 2 commits June 29, 2020 10:58

Merge branch 'dev' of https://github.com/ahamad-MS/azure-cli into dev

61ae5ac

Remove private link support frome event grid module per CR comments

c7e6844

mmyyrroonn approved these changes Jun 30, 2020

View reviewed changes

Juliehzl reviewed Jul 1, 2020

View reviewed changes

src/azure-cli/azure/cli/command_modules/eventgrid/_params.py Show resolved Hide resolved

Juliehzl reviewed Jul 1, 2020

View reviewed changes

src/azure-cli/azure/cli/command_modules/eventgrid/custom.py Outdated Show resolved Hide resolved

Ashraf Hamad added 2 commits July 1, 2020 11:43

address CR comments

e052f65

Merge remote-tracking branch 'upstream/dev' into dev

8f4319e

mmyyrroonn approved these changes Jul 2, 2020

View reviewed changes

Juliehzl mentioned this pull request Jul 3, 2020

[Core] Fix get_token() issue in msi login and expiresIn key error in cloud shell login credentials for track 2 SDK related commands #14187

Merged

2 tasks

mmyyrroonn reviewed Jul 6, 2020

View reviewed changes

Ashraf Hamad added 2 commits July 6, 2020 02:09

address CR comments

ba12753

add confirmation=true to all new delete commands

c69caa0

haroldrandom changed the title ~~[Azure Event Grid]: Add CLI support for 2020-04-01-preview and mark preview features with is_Preview=True~~ [Azure Event Grid] Add CLI support for 2020-04-01-preview and mark preview features with is_Preview=True Jul 6, 2020

haroldrandom changed the title ~~[Azure Event Grid] Add CLI support for 2020-04-01-preview and mark preview features with is_Preview=True~~ [Event Grid] Add CLI support for 2020-04-01-preview and mark preview features with is_Preview=True Jul 6, 2020

haroldrandom changed the title ~~[Event Grid] Add CLI support for 2020-04-01-preview and mark preview features with is_Preview=True~~ [EventGrid] Add CLI support for 2020-04-01-preview and mark preview features with is_Preview=True Jul 6, 2020

haroldrandom merged commit 2c3a93d into Azure:dev Jul 6, 2020

ahamad-MS deleted the dev branch March 19, 2021 16:24

Conversation

ahamad-MS commented Jun 18, 2020

Uh oh!

yonzhan commented Jun 19, 2020

Uh oh!

ahamad-MS commented Jun 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmyyrroonn commented Jun 22, 2020

Uh oh!

azure-pipelines bot commented Jun 22, 2020

Uh oh!

mmyyrroonn commented Jun 22, 2020

Uh oh!

haroldrandom commented Jun 22, 2020

Uh oh!

ahamad-MS commented Jun 22, 2020

Uh oh!

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

Uh oh!

Uh oh!

ahamad-MS commented Jun 23, 2020

Uh oh!

ahamad-MS commented Jun 29, 2020

Uh oh!

ahamad-MS commented Jun 30, 2020

Uh oh!

yonzhan commented Jun 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahamad-MS Jul 1, 2020 • 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!

ahamad-MS Jul 1, 2020 • 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!

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!

Uh oh!

ahamad-MS commented Jul 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmyyrroonn left a comment

Choose a reason for hiding this comment

Uh oh!

yonzhan commented Jul 3, 2020

Uh oh!

Juliehzl commented Jul 3, 2020

Uh oh!

ahamad-MS commented Jul 3, 2020

Uh oh!

ahamad-MS commented Jul 5, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahamad-MS commented Jul 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

ahamad-MS commented Jun 19, 2020 •

edited

Loading

haroldrandom commented Jun 22, 2020 •

edited

Loading

ahamad-MS Jul 1, 2020 •

edited

Loading

ahamad-MS Jul 1, 2020 •

edited

Loading

ahamad-MS commented Jul 1, 2020 •

edited

Loading