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

ServiceTagsListResult POCO has wrong field types #46767

Closed
kennyvanle opened this issue Oct 22, 2024 · 6 comments · Fixed by #47527
Closed

ServiceTagsListResult POCO has wrong field types #46767

kennyvanle opened this issue Oct 22, 2024 · 6 comments · Fixed by #47527
Assignees
Labels
Mgmt This issue is related to a management-plane library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Network question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team.

Comments

@kennyvanle
Copy link

Type of issue

Code doesn't work

Description

I am trying to deserialize the ServiceTagsListResult object from service tag downloadable files here: aka.ms/servicetags

The POCOs listed in the documentation and in code for the ServiceTagsListResult and the child objects are listing all of the numeric fields as strings. For example:

Image

source code

Image

This is causing an issue when deserializing the IJsonModel object using the following recommended patterns from the azure github sample ModelReaderWriter.md

Image

This is an issue as System.Json.Text won't allow us to convert Numbers -> String without manual converters, but the usage of JsonModelConverter causes all other converters to be ignored so we cannot use a custom converter on top of this.

My request is to update the POCOs with the proper type to match the json objects stored in the service tags.

Page URL

https://learn.microsoft.com/en-us/dotnet/api/azure.resourcemanager.network.models.servicetagslistresult?view=azure-dotnet

Content source URL

https://github.com/Azure/azure-docs-sdk-dotnet/blob/master/xml/Azure.ResourceManager.Network.Models/ServiceTagsListResult.xml

Document Version Independent Id

fdb8b0d7-0fc5-a5c2-8f88-265f19f77e38

Article author

@azure-sdk

Metadata

  • ID: 45527980-bb82-72a6-772c-f1df8c505451
  • Service: azure
@github-actions github-actions bot added customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Oct 22, 2024
@jsquire jsquire added Mgmt This issue is related to a management-plane library. and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Oct 22, 2024
@github-actions github-actions bot added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Oct 22, 2024
@jsquire
Copy link
Member

jsquire commented Oct 22, 2024

Thank you for your feedback. Tagging and routing to the team member best able to assist.

@ArcturusZhang
Copy link
Member

ArcturusZhang commented Oct 23, 2024

Hi @kennyvanle we would like to know where the issue was.
This property string ChangeNumber aligns with the API spec of network RP, but there is possibility that this API spec does not align with the actual traffic.
Could you attach some your code for us to reproduce the issue? And if the issue happens when you call the GetServiceTag, it would help even more if you could attach how the payload looks like.
To get the payload, you could either inspect the payload using something like fiddler or turn on the verbose traffic log in azure by following this: https://github.com/Azure/azure-sdk-for-net/blob/412c027f1d7ed8cb9675f506d2f863184c55dda2/sdk/core/Azure.Core/README.md#setting-up-console-logging

FYI I tried to call this, but since my test subscription actually does not have a thing, therefore it always gives me nothing in the result therefore I cannot reproduce, and that is why your request log could be important.

@kennyvanle
Copy link
Author

@ArcturusZhang i am actually reading the data from service tag downloadable files here: aka.ms/servicetags which contains the JSONs. From API response it looks okay. Seems like the issue is with whenever the file is getting published

Image

@ArcturusZhang
Copy link
Member

ArcturusZhang commented Nov 1, 2024

Thank you @kennyvanle . I see.
In this downloadable JSON, changeNumber appears to be a number, which might be incorrect.
I have tagged this issue so that the corresponding team should pay attention to.
In the meantime, I think it is no harm that we could introduce some flexibility into the deserialization so that it can handle both string and numbers. But since this could be a workaround, I would like to hold it until I get a response from the owner of this MS learn page.

@ArcturusZhang ArcturusZhang added the Service Attention Workflow: This issue is responsible by Azure service team. label Nov 1, 2024
@kennyvanle
Copy link
Author

Thank you! Please keep me updated. Updating the deserialization to handle both would be great since there are limitations with adding custom converters.

@kennyvanle
Copy link
Author

Hey @ArcturusZhang any update on this?

@ArthurMa1978 ArthurMa1978 removed the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Dec 6, 2024
HarveyLink added a commit that referenced this issue Dec 16, 2024
* Fix and add test

* Update CHANGELOG.md

* Update Azure.ResourceManager.Network.csproj

* Update UnitTest.cs

* Update sdk/network/Azure.ResourceManager.Network/tests/Tests/UnitTest.cs

Co-authored-by: Dapeng Zhang <ufo54153@gmail.com>

* Update sdk/network/Azure.ResourceManager.Network/tests/Tests/UnitTest.cs

Co-authored-by: Dapeng Zhang <ufo54153@gmail.com>

---------

Co-authored-by: Dapeng Zhang <ufo54153@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mgmt This issue is related to a management-plane library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Network question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants