Skip to content

CloudEvents/EG Events must recognize magic events - #19922

Merged
Rakshith Bhyravabhotla (rakshith91) merged 23 commits into
Azure:mainfrom
rakshith91:magic_events
Aug 4, 2021
Merged

CloudEvents/EG Events must recognize magic events#19922
Rakshith Bhyravabhotla (rakshith91) merged 23 commits into
Azure:mainfrom
rakshith91:magic_events

Conversation

@rakshith91

@rakshith91 Rakshith Bhyravabhotla (rakshith91) commented Jul 23, 2021

Copy link
Copy Markdown
Contributor

Fixes #19247

Comment thread sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py Outdated
Comment thread sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py Outdated
Comment thread sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py Outdated
@rakshith91 Rakshith Bhyravabhotla (rakshith91) changed the title Draft for magic events CloudEvents/EG Events must recognize magic events Jul 27, 2021
Comment thread sdk/core/azure-core/CHANGELOG.md Outdated
Comment thread sdk/core/azure-core/azure/core/messaging.py Outdated
Comment thread sdk/core/azure-core/azure/core/messaging.py Outdated
Comment thread sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py Outdated
# eventhubs
try:
return json.loads(next(obj.body))[0]
except KeyError:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it possible an IndexError could be raised here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And in what case would it raise a KeyError?
Do we also need different handling for a StopIteration error? In that scenario, it meant that we had been passed the right object, but the payload was not valid - so I would expect that to then raise a ValueError, no?

@rakshith91 Rakshith Bhyravabhotla (rakshith91) Aug 2, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is it possible an IndexError could be raised here?

I cannot think of a scenario which could raise IndexError, and in case it is, we still return the object as is - and i have tried sending events to EH in diffferent formats - it always returns a "list"

And in what case would it raise a KeyError?

A keyerror is raised when we try to subscript a SB message with 0. essentially, service bus message is a dict. which is fetched by json.loads(next(obj.body)). So doing a json.loads(next(obj.body))[0] will give you a key error.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do we also need different handling for a StopIteration error
No - uamqp does't throw stop iteration https://github.com/Azure/azure-uamqp-python/blob/master/uamqp/message.py#L1159-L1162

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Interesting - so if the Event body is an empty list - calling next on it doesn't raise? What does it do?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

m = message.Message(body= b'', body_type=uamqp.MessageBodyType.Data)
for data in m.get_data():
  print(data)

>>> b''

Comment thread sdk/core/azure-core/azure/core/messaging.py Outdated
Comment thread sdk/eventgrid/azure-eventgrid/azure/eventgrid/_models.py Outdated
Comment thread sdk/core/azure-core/azure/core/messaging.py Outdated
Comment thread sdk/core/azure-core/azure/core/messaging.py
Comment thread sdk/core/azure-core/azure/core/utils/_utils.py Outdated
Comment thread sdk/core/azure-core/azure/core/utils/_utils.py Outdated
Comment thread sdk/core/azure-core/tests/test_messaging_cloud_event.py
Comment thread sdk/eventgrid/azure-eventgrid/azure/eventgrid/_helpers.py Outdated
@rakshith91

Copy link
Copy Markdown
Contributor Author

/check-enforcer override

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CloudEvent should magically recognize dict's from known services

2 participants