[MetricsAdvisor] Bug fix: service returns null in unexpected scenarios #17776
Conversation
kinelski
added
Client
| @@ -83,6 +83,113 @@ directive: | |||
| $.properties.upperBoundaryList.items["x-nullable"] = true; | |||
| ``` | |||
|
|
|||
| ``` yaml | |||
There was a problem hiding this comment.
Is there an Issue we can link to that tracks these issues with the swagger?
There was a problem hiding this comment.
Nope, but I can create a new one. I think we'll never close it, though.
There was a problem hiding this comment.
Is the intention to communicate to the service about this changes and make sure they address them in their swagger?
There was a problem hiding this comment.
Yes. However, based on previous experiences in which we faced literally the same issue, this tends to be treated as a very low priority issue and ends up never being fixed (specially since we can workaround it on the client side). You can see we already had a set of similar transforms in MA for the same reason (service team already aware).
| @@ -43,7 +43,7 @@ internal HttpRequestDataFeedSource(HttpRequestParameter parameter) | |||
|
|
|||
| Parameter = parameter; | |||
|
|
|||
| Url = new Uri(parameter.Url); | |||
| Url = parameter.Url == null ? null : new Uri(parameter.Url); | |||
There was a problem hiding this comment.
Is there a test covering this change?
There was a problem hiding this comment.
When making a
GetDataFeedrequest to the service, the parameters of the returned Data Source will benullif the requester doesn't have an Administrator role. The corresponding properties are not marked withx-nullable = truein the swagger, so we're throwing aNullReferenceExceptionin these scenarios.This PR adds transforms to
autorest.mdand regenerates the code to fix this issue.