Skip to content

Xml support#2806

Merged
msyyc merged 31 commits intomainfrom
xml_support
Sep 9, 2024
Merged

Xml support#2806
msyyc merged 31 commits intomainfrom
xml_support

Conversation

@tadelesh
Copy link
Copy Markdown
Member

@tadelesh tadelesh commented Sep 2, 2024

No description provided.

} else {
xmlMetadata["text"] = true;
}
break;
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.

Lack default for switch

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

default do nothing.

@tadelesh tadelesh marked this pull request as ready for review September 5, 2024 07:30
@tadelesh tadelesh requested a review from iscai-msft as a code owner September 5, 2024 07:30
Copy link
Copy Markdown
Contributor

@iscai-msft iscai-msft left a comment

Choose a reason for hiding this comment

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

thanks so much for your work @tadelesh

if (type.kind === "property" && type.type.kind === "array" && type.type.valueType.kind !== "model") {
const itemMetadata = getXmlMetadata(type.type.valueType);
// if array item is a primitive type, we need to use itemsName to change the name
if (Object.keys(itemMetadata).length > 0) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: should these just correspond 1-1 with what tsp is returning to us? I.e. xmlMetadata["name"] = itemMetadata["name"]

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

tsp only returns the user defined decorator data. xml has some convention that i need to specific handle. so the item related metadata is only for ser/deser in python.

for _, rf in self._attr_to_rest_field.items():
prop_meta = getattr(rf, "_xml", {})
xml_name = prop_meta.get("name", rf._rest_name)
xml_ns = prop_meta.get("ns", model_meta.get("ns", None))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i'm not familiar enough with all of this code, but I just want to make sure that we generate everything when we output our generated models. Like how right now we aren't always generating a deserializing callback, even though during generation time we know exactly what the deserializing callback should be. So we can generate xml stuff all the time, so we don't have to guess

Copy link
Copy Markdown
Member Author

@tadelesh tadelesh Sep 6, 2024

Choose a reason for hiding this comment

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

current logic for xml does not touch any deserialization mapping in __new__. so what you want is a common logic both for json and xml. i saw you opened this: #2801. i will work on the optimization after vacation.

@tadelesh
Copy link
Copy Markdown
Member Author

tadelesh commented Sep 6, 2024

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@msyyc msyyc enabled auto-merge (squash) September 9, 2024 06:10
@msyyc msyyc merged commit 10d8e5b into main Sep 9, 2024
@msyyc msyyc deleted the xml_support branch September 9, 2024 08:43
@tadelesh tadelesh mentioned this pull request Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants