Remove System Events #16358
Remove System Events #16358
Conversation
rakshith91
requested review from
lmazuel,
swathipil and
yunhaoling
as code owners
ghost
added
the
|
/azp run python - eventgrid - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - eventgrid - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
swathipil
left a comment
swathipil
left a comment
There was a problem hiding this comment.
left a few small comments that you may already have already thought of or have plans to change in other PRs. pls lmk if this is the case, I can approve!
| """ | ||
| encode = kwargs.pop('encoding', 'utf-8') | ||
| try: | ||
| cloud_event = CloudEvent._from_json(cloud_event, encode) # pylint: disable=protected-access |
There was a problem hiding this comment.
this will need updating I believe once you change the _from_json to _to_json :)
There was a problem hiding this comment.
so I just realize that currently the _from_json accepts a json string and returns a json/dict object?
and if I'm understanding correctly, we're gonna have a separate PR which has the methods matching the functionality?
(probably we could be a bit verbose on the method name, like _from_json_str, _to_json_object-ish)
| """ | ||
| encode = kwargs.pop('encoding', 'utf-8') | ||
| try: | ||
| eventgrid_event = EventGridEvent._from_json(eventgrid_event, encode) # pylint: disable=protected-access |
There was a problem hiding this comment.
same as above, will need updating once _from_json is changed _to_json
There was a problem hiding this comment.
I have this tracked in the decode changes where i'm including a _to_json as well (along with a _from_json) #16352
| """ | ||
| import json | ||
| from azure.eventgrid import EventGridDeserializer, EventGridEvent | ||
| from azure.eventgrid.systemevents import StorageBlobCreatedEventData |
There was a problem hiding this comment.
because the _deserialize_data method will be removed from EventMixin, it might be useful to add samples (or add within this sample) of how SystemEventMappings can be used with the data/type after deserializing cloud/eg events. (if you already have something like this in the works, ignore this comment :) )
There was a problem hiding this comment.
same here - the samples will significantly change in the above issue i mentioned - so will update there
Co-authored-by: Adam Ling (MSFT) <[email protected]>
3 participants
Fixes #16351