Skip to content
This repository was archived by the owner on Apr 10, 2024. It is now read-only.

Add 'innererror' property bag to CloudErrorData contract#73

Merged
lmazuel merged 2 commits intoAzure:masterfrom
ilidemi:clouderror-innererror
Jan 5, 2018
Merged

Add 'innererror' property bag to CloudErrorData contract#73
lmazuel merged 2 commits intoAzure:masterfrom
ilidemi:clouderror-innererror

Conversation

@ilidemi
Copy link
Copy Markdown
Contributor

@ilidemi ilidemi commented Jan 3, 2018

The data contract for ARM errors includes innererror field which is a property bag with unspecified contents as per OData 4.0 spec. This field wasn't used by ARM up until now, but recently there was a change related to Azure Policy that makes heavy use of this property bag. It is already deployed in all regions and consumed by Azure Portal, so now would be a good time to support it in SDKs as well.

The new errors are generated by the ARM Frontdoor itself, so CloudErrorData seems like the most appropriate contract to include it.

Copy link
Copy Markdown
Member

@lmazuel lmazuel left a comment

Choose a reason for hiding this comment

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

Just add a test please and we're good to go.
Thanks for the contribution!

cloud_exp = self._d(CloudErrorData(), message)
self.assertEqual(cloud_exp.target, 'query')
self.assertEqual(cloud_exp.details[0].target, '$search')
self.assertEqual(cloud_exp.innererror['customKey'], 'customValue')
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.

Could you add a str call? You're not testing the __str__ change

self.assertIn("customValue", str(cloud_exp))

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.

Updated the test. Thanks for the quick review!

@lmazuel lmazuel merged commit d44ebe9 into Azure:master Jan 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants