Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement from_dict #16509

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions sdk/eventgrid/azure-eventgrid/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## 2.0.0b5 (Unreleased)

**Features**
- `CloudEvent` and `EventGridEvent` now have a `from_dict` method which deserializes a dict representation into corresponding object.

**Breaking Changes**
- `EventGridSharedAccessSignatureCredential` is deprecated in favor of `AzureSasCredential`.
- `azure.eventgrid.models` namespace along with all the models in it are now removed. `azure.eventgrid.SystemEventNames` can be used to get the event model type mapping.
Expand Down
9 changes: 9 additions & 0 deletions sdk/eventgrid/azure-eventgrid/azure/eventgrid/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,12 @@ def _eventgrid_data_typecheck(event):
if isinstance(data, six.binary_type):
raise TypeError("Data in EventGridEvent cannot be bytes. Please refer to"\
"https://docs.microsoft.com/en-us/azure/event-grid/event-schema")

def _decode(content):
try:
return base64.b64decode(content.encode('utf-8'))
Copy link
Contributor

@yunhaoling yunhaoling Feb 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dumb question: what does the content look like, utf-8 base64 string?
also probably worth a better method name _decode_base64_string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"data_base64":'Y2xvdWRldmVudA==',

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we decided not to expose data_base64 to users at all
see #16661

except (ValueError, TypeError) as error:
# ValueError for Python 3, TypeError for Python 2
raise ValueError(
message="Data is not valid base 64.",
error=error)
49 changes: 49 additions & 0 deletions sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import json
import six
from msrest.serialization import UTC
from ._helpers import _decode
from ._generated.models import EventGridEvent as InternalEventGridEvent, CloudEvent as InternalCloudEvent


Expand Down Expand Up @@ -106,6 +107,31 @@ def __init__(self, source, type, **kwargs): # pylint: disable=redefined-builtin
raise ValueError("data and data_base64 cannot be provided at the same time.\
Use data_base64 only if you are sending bytes, and use data otherwise.")

@classmethod
def from_dict(cls, event, **kwargs):
# type: (Dict, Any) -> CloudEvent
"""
Returns the deserialized CloudEvent object when a dict is provided.

:param event: The dict representation of the event which needs to be deserialized.
:type event: dict
Comment on lines +114 to +117
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be helpful if we could add description on the key-value items we accepts -- but it's a doc stuff

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, the payload normally comes directly from a storage queue - i can put a link to the relevant spec

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not all the key-value pairs, at least mentioning that data_base64 will always be decoded and set as the value for the data field would be helpful. In general, maybe mentioning that from_dict will never result in the data_base64 field being set in the resulting CloudEvent object (if I'm getting that right).


:rtype: CloudEvent
"""
return cls(
id=event.pop("id", None),
source=event.pop("source", None),
type=event.pop("type", None),
specversion=event.pop("specversion", None),
data=event.pop("data", None) or _decode(event.pop("data_base64", None)),
Copy link
Contributor

@yunhaoling yunhaoling Feb 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my understanding, to use the from_dict for CloudEvent, users should be doing this:

dict = {
    data_base64 = b'<base64 bytes>'
}

# or

dict = {
    data = data
}

which means internally we do:

data=event.pop("data", None)
data_base64=event.pop("data_base64", None),

is setting input data_base64 to the data argument by design?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is by design, and mirrors this. Similar to sending data that is of bytes type (sent as data_base64 in the outgoing request), when from_dict is used to deserialize, it sets data_base64 back to data.

Copy link
Contributor

@yunhaoling yunhaoling Feb 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking of the following two creation case which makes me feel the inconsistency, from intuition I would expect two cases to return the "same" CloudEvent :

event_created_from_class = CloudEvent(data_base64=b'bytes_data')
event_created_from_class.data is None
event_created_from_class.data_base64 is not None

event_created_from_dict = CloudEvent.from_dict({'data_base64':b'bytes_data'})
event_created_from_dict.data is not None
event_created_from_dict.data_base64 is None

apart from that, I notice that from_dict could take both data and data_base64, and if both are set data_base64 gets ignored:

# I think this should be an invalid input?
event_created_from_dict = CloudEvent.from_dict({'data': 'somedata', 'data_base64':b'bytes_data'})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I could also be wrong, might be missing some context here, just thinking out loud)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That two ways that data_base64 can be deserialized is a little confusing since, no matter what, only data will end up being set. Maybe clarifying this in the docstring of from_dict would be useful?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could also do a cross-language check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update: we decided not to expose data_base64
#16661

time=event.pop("time", None),
dataschema=event.pop("dataschema", None),
datacontenttype=event.pop("datacontenttype", None),
subject=event.pop("subject", None),
extensions=event,
**kwargs
)

@classmethod
def _from_generated(cls, cloud_event, **kwargs):
# type: (Union[str, Dict, bytes], Any) -> CloudEvent
Expand Down Expand Up @@ -228,3 +254,26 @@ def __init__(self, subject, event_type, data, data_version, **kwargs):
kwargs.setdefault('data_version', data_version)

super(EventGridEvent, self).__init__(**kwargs)

@classmethod
def from_dict(cls, event, **kwargs):
# type: (Dict, Any) -> EventGridEvent
"""
Returns the deserialized EventGridEvent object when a dict is provided.

:param event: The dict representation of the event which needs to be deserialized.
:type event: dict

:rtype: EventGridEvent
Comment on lines +262 to +267
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as above regarding to the description of the dict accepted key-value items.

"""
return cls(
id=event.get("id", None),
subject=event.get("subject", None),
topic=event.get("topic", None),
data_version=event.get("dataVersion", None),
data=event.get("data", None),
event_time=event.get("eventTime", None),
event_type=event.get("eventType", None),
metadata_version=event.get("metadataVersion", None),
**kwargs
)
40 changes: 14 additions & 26 deletions sdk/eventgrid/azure-eventgrid/tests/_mocks.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
import json


# storage cloud event
cloud_storage_dict = {
"id":"a0517898-9fa4-4e70-b4a3-afda1dd68672",
Expand All @@ -21,20 +18,28 @@
"time":"2020-08-07T01:11:49.765846Z",
"specversion":"1.0"
}
cloud_storage_string = json.dumps(cloud_storage_dict)
cloud_storage_bytes = cloud_storage_string.encode("utf-8")

# custom cloud event
cloud_custom_dict = {
cloud_custom_dict_base64 = {
"id":"de0fd76c-4ef4-4dfb-ab3a-8f24a307e033",
"source":"https://egtest.dev/cloudcustomevent",
"data":{"team": "event grid squad"},
"data_base64":'Y2xvdWRldmVudA==',
"type":"Azure.Sdk.Sample",
"time":"2020-08-07T02:06:08.11969Z",
"specversion":"1.0"
}
cloud_custom_string = json.dumps(cloud_custom_dict)
cloud_custom_bytes = cloud_custom_string.encode("utf-8")

cloud_custom_dict_with_extensions = {
"id":"de0fd76c-4ef4-4dfb-ab3a-8f24a307e033",
"source":"https://egtest.dev/cloudcustomevent",
"data":{"team": "event grid squad"},
"type":"Azure.Sdk.Sample",
"time":"2020-08-07T02:06:08.11969Z",
"specversion":"1.0",
"ext1": "example",
"ext2": "example2"
}


# storage eg event
eg_storage_dict = {
Expand All @@ -58,20 +63,3 @@
"eventTime":"2020-08-07T02:28:23.867525Z",
"topic":"/subscriptions/faa080af-c1d8-40ad-9cce-e1a450ca5b57/resourceGroups/t-swpill-test/providers/Microsoft.EventGrid/topics/eventgridegsub"
}

eg_storage_string = json.dumps(eg_storage_dict)
eg_storage_bytes = eg_storage_string.encode("utf-8")

# custom eg event
eg_custom_dict = {
"id":"3a30afef-b604-4b67-973e-7dfff7e178a7",
"subject":"Test EG Custom Event",
"data":{"team":"event grid squad"},
"eventType":"Azure.Sdk.Sample",
"dataVersion":"2.0",
"metadataVersion":"1",
"eventTime":"2020-08-07T02:19:05.16916Z",
"topic":"/subscriptions/f8aa80ae-d1c8-60ad-9bce-e1a850ba5b67/resourceGroups/sample-resource-group-test/providers/Microsoft.EventGrid/topics/egtopicsamplesub"
}
eg_custom_string = json.dumps(eg_custom_dict)
eg_custom_bytes = eg_custom_string.encode("utf-8")
56 changes: 56 additions & 0 deletions sdk/eventgrid/azure-eventgrid/tests/test_from_dict.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import logging
import sys
import os
import pytest
import json

from azure.eventgrid import CloudEvent, EventGridEvent
from devtools_testutils import AzureMgmtTestCase
from _mocks import (
cloud_storage_dict,
cloud_custom_dict_base64,
cloud_custom_dict_with_extensions,
eg_storage_dict
)

class EventGridDeserializerTests(AzureMgmtTestCase):

# Cloud Event tests
def test_eg_consumer_cloud_storage_dict(self, **kwargs):
event = CloudEvent.from_dict(cloud_storage_dict)
assert event.data == {
"api":"PutBlockList",
"client_request_id":"6d79dbfb-0e37-4fc4-981f-442c9ca65760",
"request_id":"831e1650-001e-001b-66ab-eeb76e000000",
"e_tag":"0x8D4BCC2E4835CD0",
"content_type":"application/octet-stream",
"content_length":524288,
"blob_type":"BlockBlob",
"url":"https://oc2d2817345i60006.blob.core.windows.net/oc2d2817345i200097container/oc2d2817345i20002296blob",
"sequencer":"00000000000004420000000000028963",
"storage_diagnostics":{"batchId":"b68529f3-68cd-4744-baa4-3c0498ec19f0"}
}
assert event.specversion == "1.0"
assert event.__class__ == CloudEvent


def test_cloud_custom_dict_with_extensions(self, **kwargs):
event = CloudEvent.from_dict(cloud_custom_dict_with_extensions)
assert event.data == {"team": "event grid squad"}
assert event.__class__ == CloudEvent
assert event.extensions == {"ext1": "example", "ext2": "example2"}

def test_cloud_custom_dict_base64(self, **kwargs):
event = CloudEvent.from_dict(cloud_custom_dict_base64)
assert event.data == b'cloudevent'
assert event.data_base64 == None
Comment on lines +45 to +46
Copy link
Contributor

@yunhaoling yunhaoling Feb 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I mentioned in the from_dict, the dictionary item is data_base64 while we actually set it to the data which I think kinda breaks consistency from the experience of using the client constructor.

assert event.specversion == "1.0"
assert event.__class__ == CloudEvent

# EG Event tests

def test_eg_storage_from_dict(self, **kwargs):
event = EventGridEvent.from_dict(eg_storage_dict)
assert event.__class__ == EventGridEvent
assert event.event_time == "2020-08-07T02:28:23.867525Z"
assert event.event_type == "Microsoft.Storage.BlobCreated"