Arch preview feedback #16441

rakshith91 · 2021-02-01T07:04:04Z

Fixes #16433

Kill Custom Events
Kill Deserializer
Cloud Event make data optional
Support Data / data_base64 (not both)data must be optional ; data_base64 is optional too data can be bytes(convert to data_base64) data and data_base64 at the same time raises.
SystemEventMappings to SystemEvents
change order of EG event

rakshith91 · 2021-02-02T17:31:49Z

/azp run python - eventgrid - tests

azure-pipelines · 2021-02-02T17:31:59Z

Azure Pipelines successfully started running 1 pipeline(s).

rakshith91 · 2021-02-02T18:12:12Z

/azp run python - eventgrid - tests

azure-pipelines · 2021-02-02T18:12:22Z

Azure Pipelines successfully started running 1 pipeline(s).

check-enforcer · 2021-02-02T18:17:33Z

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

What if I am onboarding a new service?

Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment:
/azp run prepare-pipelines
This will run a pipeline that analyzes the source tree and creates the pipelines necessary to build and validate your pull request. Once the pipeline has been created you can trigger the pipeline using the following comment:
/azp run python - [service] - ci

rakshith91 · 2021-02-02T18:17:50Z

/check-enforcer evaluate

sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py

yunhaoling · 2021-02-02T23:07:12Z

sdk/eventgrid/azure-eventgrid/tests/test_eg_publisher_client.py

+        cloud_event = CloudEvent(
+                source = "http://samplesource.dev",
+                data = b'cloudevent',
+                type="Sample.Cloud.Event"
+                )
+
+        def callback(request):
+            req = json.loads(request.http_request.body)
+            assert req[0].get("data_base64") is not None
+            assert req[0].get("data") is None


now I'm getting a bit confused by this request body.. because you set the data in the CloudEvent while the http request body get("data") is None,

so if I pass bytes to the data argument, which will internally set the data_base64 variable?

cloud_event = CloudEvent( data = b'cloudevent', ) # which of the following will have the value set? cloud_event.data cloud_event.data_base64

or it's just implementation detail that users don't need to care about.

internally, cloud_event.data_base64 is set when we we pass bytes regardless of what we use in data, data_base64 to send bytes.

Based on the above, it might be useful to add a sample where you deserialize the CloudEvent that has the data field set to a binary value. Then, after deserializing, call cloud_event.data_base64 and comment that it returns a not None value. Like this sample, except with the data field set to b'cloudevent' like the test above.

we do not have a from_dict implementation yet - it'd make more sense to add the samples after we have a symmetric workflow (plus i would like to unblock the breaking changes first)

sdk/eventgrid/azure-eventgrid/azure/eventgrid/aio/_publisher_client_async.py

sdk/eventgrid/azure-eventgrid/samples/champion_scenarios.md

sdk/eventgrid/azure-eventgrid/azure/eventgrid/aio/_publisher_client_async.py

sdk/eventgrid/azure-eventgrid/samples/champion_scenarios.md

Co-authored-by: Adam Ling (MSFT) <adam_ling@outlook.com>

sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py

swathipil

One more thing: you probably will need to update the samples/README.md file to reflect the changes here (which you may already be planning on doing).

sdk/eventgrid/azure-eventgrid/README.md

swathipil · 2021-02-03T00:11:31Z

sdk/eventgrid/azure-eventgrid/README.md

-
-### EventGridDeserializer
-`EventGridDeserializer` is used to desrialize an event received.
+Either a list or a single instance of CloudEvent/EventGridEvent can be sent. Alternatively, a list or a single instance of the dict representation of CloudEvent/EventGridEvent or a Custom Schema can also be published.


The wording is a little confusing. Might be more clear to separate the idea that CloudEvent/EGEvents can be sent as dicts and that events in a custom schema can be sent as a dict or list of dicts. Would be useful to add links for either the samples of sending all these different formats or from the champion_scenarios.md . Something like?

Suggested change

Either a list or a single instance of CloudEvent/EventGridEvent can be sent. Alternatively, a list or a single instance of the dict representation of CloudEvent/EventGridEvent or a Custom Schema can also be published.

The following formats of events are allowed to be sent:

- a [list of strongly-typed EventGridEvents](add link to list of strongly typed EGE from champion_scenarios.md)

- a [list of strongly-typed CloudEvents](add link to scenario from champion_scenarios.md)

- a [single strongly-typed EventGridEvent instance](add link to single strongly typed EGE from champion_scenarios.md)

- a [single strongly-typed CloudEvent instance](add link to scenario from champion_scenarios.md)

- a list of the [dict representation of EventGridEvents](add link)

- a list of the [dict representation of CloudEvents](add link)

- a single instance of the [dict representation of EventGridEvent](url)

- a single instance of the [dict representation of CloudEvent](url)

- a list of [dicts in a custom schema](link)

- a single [dict in a custom schema](url)

it would be worth it - i'll tackle it in the samples issue though since i fell this is a non blocker. changing the wording a bit for now

swathipil · 2021-02-03T00:14:55Z

sdk/eventgrid/azure-eventgrid/samples/champion_scenarios.md

+
+for event in deserialized_dict_events:
+    print(event.data)
+    print(type(event))


out of curiosity, did you run all these samples outlined here? Or did you copy them from the actual samples and change things to fit each of the scenarios?

good questions - if you noticed, the from_dict isn't implemented yet :P - other than the two of those, everything else is running and works fine. this file is like a quick reference till i add more relevant samples

swathipil · 2021-02-03T00:47:51Z

sdk/eventgrid/azure-eventgrid/tests/test_eg_publisher_client.py

+        cloud_event = CloudEvent(
+                source = "http://samplesource.dev",
+                data = b'cloudevent',
+                type="Sample.Cloud.Event"
+                )
+
+        def callback(request):
+            req = json.loads(request.http_request.body)
+            assert req[0].get("data_base64") is not None
+            assert req[0].get("data") is None


Based on the above, it might be useful to add a sample where you deserialize the CloudEvent that has the data field set to a binary value. Then, after deserializing, call cloud_event.data_base64 and comment that it returns a not None value. Like this sample, except with the data field set to b'cloudevent' like the test above.

sdk/eventgrid/azure-eventgrid/README.md

rakshith91 · 2021-02-03T01:31:09Z

Here's the queued live build https://dev.azure.com/azure-sdk/internal/_build/results?buildId=716144&view=results
the CI link: https://dev.azure.com/azure-sdk/public/_build/results?buildId=716137&view=results

sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py

Co-authored-by: Adam Ling (MSFT) <adam_ling@outlook.com>

...grid/azure-eventgrid/samples/publish_samples/publish_custom_schema_events_to_topic_sample.py

…custom_schema_events_to_topic_sample.py

rakshith91 · 2021-02-03T06:09:28Z

/azp run python - eventgrid - tests

azure-pipelines · 2021-02-03T06:09:37Z

Azure Pipelines successfully started running 1 pipeline(s).

…into enum-meta * 'master' of https://github.com/Azure/azure-sdk-for-python: bump six dependencies in some libraries (#16496) call on_error if timeout in flush (#16485) Sync eng/common directory with azure-sdk-tools for PR 1365 (#16505) Fix min dependency tests - update azure core (#16504) Sync eng/common directory with azure-sdk-tools for PR 1364 (#16503) Ma arch feedback (#16502) Adding a new limitation to the README file. (#16475) [Blob][Datalake] STG76 Preview (#16349) append code coverage over each other (#16202) Arch preview feedback (#16441) Support CAE in azure-identity (#16323) [EventHubs] Support for Custom endpoint adddress and custom certificate (#16295) [Communication] - Phone Number Management - Added support for AAD auth (#16075) fix live tests (#16495)

…into analyze_redesign * 'master' of https://github.com/Azure/azure-sdk-for-python: (32 commits) Adopt new MSAL auth code flow API (Azure#16449) [formrecognizer] use ARM template for tests (Azure#16432) T2 kusto 2021 02 04 (Azure#16527) T2 applicationinsights 2021 02 04 (Azure#16525) Sync eng/common directory with azure-sdk-tools for PR 1366 (Azure#16506) [Python] python track2 new pipeline fix (Azure#16494) Added package properties SDKType and NewSDK (Azure#16476) bump six dependencies in some libraries (Azure#16496) call on_error if timeout in flush (Azure#16485) Sync eng/common directory with azure-sdk-tools for PR 1365 (Azure#16505) Fix min dependency tests - update azure core (Azure#16504) Sync eng/common directory with azure-sdk-tools for PR 1364 (Azure#16503) Ma arch feedback (Azure#16502) Adding a new limitation to the README file. (Azure#16475) [Blob][Datalake] STG76 Preview (Azure#16349) append code coverage over each other (Azure#16202) Arch preview feedback (Azure#16441) Support CAE in azure-identity (Azure#16323) [EventHubs] Support for Custom endpoint adddress and custom certificate (Azure#16295) [Communication] - Phone Number Management - Added support for AAD auth (Azure#16075) ...

rakshith91 added 9 commits January 31, 2021 08:59

Kill Deserializer

c0be246

kill cuatom events

56e3980

cloud event

bae007d

eg event

307223d

SystemEventNames

8c9c934

more changes

f45d34c

doc string

76d0b71

aio docstring

121245b

samples

e13c9be

ghost added the Event Grid label Feb 1, 2021

rakshith91 added 2 commits January 31, 2021 23:32

lint fix

2344688

lint

ab431b4

rakshith91 marked this pull request as ready for review February 1, 2021 08:08

rakshith91 requested review from lmazuel, swathipil and yunhaoling as code owners February 1, 2021 08:08

rakshith91 and others added 3 commits February 1, 2021 12:44

updates

654f93c

Update champion_scenarios.md

9d2b2ea

updates

5b0c8e4

fix samples

71a12df

rakshith91 added 4 commits February 2, 2021 10:29

md file

7cf7c97

custom-2

4450fba

oops

6e9320f

tests

d0639bb

yunhaoling reviewed Feb 2, 2021

View reviewed changes

rakshith91 commented Feb 2, 2021

View reviewed changes

sdk/eventgrid/azure-eventgrid/azure/eventgrid/aio/_publisher_client_async.py Outdated Show resolved Hide resolved

rakshith91 commented Feb 3, 2021

View reviewed changes

sdk/eventgrid/azure-eventgrid/samples/champion_scenarios.md Outdated Show resolved Hide resolved

Apply suggestions from code review

da3d505

Co-authored-by: Adam Ling (MSFT) <adam_ling@outlook.com>

yunhaoling reviewed Feb 3, 2021

View reviewed changes

sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py Outdated Show resolved Hide resolved

swathipil reviewed Feb 3, 2021

View reviewed changes

change

7559f61

rakshith91 commented Feb 3, 2021

View reviewed changes

sdk/eventgrid/azure-eventgrid/README.md Outdated Show resolved Hide resolved

Rakshith Bhyravabhotla and others added 2 commits February 2, 2021 17:02

Apply suggestions from code review

78aebb7

lint

2cb0908

fix sample

e0e78c2

yunhaoling reviewed Feb 3, 2021

View reviewed changes

sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py Outdated Show resolved Hide resolved

Update sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py

6c0c6bb

Co-authored-by: Adam Ling (MSFT) <adam_ling@outlook.com>

rakshith91 commented Feb 3, 2021

View reviewed changes

...grid/azure-eventgrid/samples/publish_samples/publish_custom_schema_events_to_topic_sample.py Outdated Show resolved Hide resolved

Update sdk/eventgrid/azure-eventgrid/samples/publish_samples/publish_…

9ecbada

…custom_schema_events_to_topic_sample.py

rakshith91 enabled auto-merge (squash) February 3, 2021 06:48

yunhaoling approved these changes Feb 3, 2021

View reviewed changes

rakshith91 disabled auto-merge February 3, 2021 18:48

rakshith91 merged commit fb91388 into Azure:master Feb 3, 2021

swathipil mentioned this pull request Feb 4, 2021

Implement from_dict #16509

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Arch preview feedback #16441

Arch preview feedback #16441

rakshith91 commented Feb 1, 2021 •

edited

Loading

rakshith91 commented Feb 2, 2021

azure-pipelines bot commented Feb 2, 2021

rakshith91 commented Feb 2, 2021

azure-pipelines bot commented Feb 2, 2021

check-enforcer bot commented Feb 2, 2021

rakshith91 commented Feb 2, 2021

yunhaoling Feb 2, 2021

rakshith91 Feb 3, 2021

swathipil Feb 3, 2021

rakshith91 Feb 3, 2021

swathipil left a comment

swathipil Feb 3, 2021

rakshith91 Feb 3, 2021

swathipil Feb 3, 2021

rakshith91 Feb 3, 2021

swathipil Feb 3, 2021

rakshith91 commented Feb 3, 2021 •

edited

Loading

rakshith91 commented Feb 3, 2021

azure-pipelines bot commented Feb 3, 2021

-Either a list or a single instance of CloudEvent/EventGridEvent can be sent. Alternatively, a list or a single instance of the dict representation of CloudEvent/EventGridEvent or a Custom Schema can also be published.
+The following formats of events are allowed to be sent:
+- a [list of strongly-typed EventGridEvents](add link to list of strongly typed EGE from champion_scenarios.md)
+- a [list of strongly-typed CloudEvents](add link to scenario from champion_scenarios.md)
+- a [single strongly-typed EventGridEvent instance](add link to single strongly typed EGE from champion_scenarios.md)
+- a [single strongly-typed CloudEvent instance](add link to scenario from champion_scenarios.md)
+- a list of the [dict representation of EventGridEvents](add link)
+- a list of the [dict representation of CloudEvents](add link)
+- a single instance of the [dict representation of EventGridEvent](url)
+- a single instance of the [dict representation of CloudEvent](url)
+- a list of [dicts in a custom schema](link)
+- a single [dict in a custom schema](url)

Arch preview feedback #16441

Arch preview feedback #16441

Conversation

rakshith91 commented Feb 1, 2021 • edited Loading

rakshith91 commented Feb 2, 2021

azure-pipelines bot commented Feb 2, 2021

rakshith91 commented Feb 2, 2021

azure-pipelines bot commented Feb 2, 2021

check-enforcer bot commented Feb 2, 2021

What is Check Enforcer?

Why am I getting this message?

What should I do now?

What if I am onboarding a new service?

rakshith91 commented Feb 2, 2021

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

swathipil left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakshith91 commented Feb 3, 2021 • edited Loading

rakshith91 commented Feb 3, 2021

azure-pipelines bot commented Feb 3, 2021

rakshith91 commented Feb 1, 2021 •

edited

Loading

rakshith91 commented Feb 3, 2021 •

edited

Loading