Add smithy-xml package#655
Add smithy-xml package#655arandito wants to merge 2 commits intosmithy-lang:feat/add-query-protocolfrom
Conversation
|
Update: XML serializers are omitted from this initial implementation as Reviewer NoteThe current XML serializer builds a DOM tree in memory and only writes it to the output sink when I explored implementing a streaming XML writer to avoid this but ran into an issue with XML attributes. In XML, members marked with One possible solution is a two-pass approach: first serialize attribute members normally while sending others to a null serializer, then serialize non-attribute members while sending attribute members to a null serializer. This allows the opening tag and its attributes to be written before child elements. However, this introduces additional complexity compared to the current approach without a substantial benefit. This creates some asymmetry with the current XML deserializer, which reads incrementally from the byte source rather than a DOM. I am open to switching the serializer to a streaming implementation if parity or avoiding the intermediate DOM is preferred. |
jonathan343
left a comment
There was a problem hiding this comment.
I'll need to go over this a second time, but dropping some quick comments now since I might not be able to get back to this soon:
| return self.document_value.get("scheme") # type: ignore | ||
|
|
||
|
|
||
| @dataclass(init=False, frozen=True) |
There was a problem hiding this comment.
I'm thinking might be worth pulling this into a separate PR that targets develop so we can get this released early. My though process is:
- If
smithy-xmlhas a dependency onsmithy-core(unbounded) we will have cases where people using versions that exist today such as (e.g., 0.3.0) which won't work. - It might be good to set a lower bound in
smithy-xmllikesmithy-core>=<version>. However, to do this we'll need a specific version we know has these traits.
In reality, people using smithy-xml will be using it through some aws client that pins the proper dependencies. So I'm not too worried about it. But also if this is in a patch bump, the aws clients do things like ~= which still makes it possible for us to have weird cases
There was a problem hiding this comment.
I think it's fine to keep here for now so the code works and tests are passing. But something we may want to consider doing in parallel and then rebase later
| def is_null(self) -> bool: | ||
| return False |
Description
This PR adds an initial implementation of the new
smithy-xmlruntime package, which provides generic XML serialization and deserialization support for Smithy clients and servers.This PR primarily implements the XML deserialization support needed by the
awsQueryprotocol. The serializer is intentionally omitted for now asawsQuerydoes not use XML request serialization. It will be introduced later alongside therestXmlprotocol, where XML request serialization is required and can be validated more thoroughly.The implementation follows the XML binding rules defined in the Smithy awsQuery and restXml protocol specifications, including support for the following traits:
@xmlName,@xmlAttribute,@xmlFlattened, and@xmlNamespace(defined insmithy-core).Important
This implementation follows the Smithy Python Protocol Serialization and Deserialization design. I strongly recommend reviewing this document first to better understand the implementation.
Testing
Added round trip serde test cases that follow the same testing strategy as
smithy-json.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.