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

detect if a type really needs ser/deserialization then generate them based on the detection #2856

Open
qiaozha opened this issue Oct 15, 2024 · 0 comments
Assignees
Labels
HRLC P1 priority 1

Comments

@qiaozha
Copy link
Member

qiaozha commented Oct 15, 2024

Currently, we will generate the ser/deserializer for a type if it's model, dict, array, or union, but we could have further check on the elmentType or union variant type or the properties of the model to see if it really needs ser/deser logic for it. Like if the properties between the rest layer and modular layer are inconsistent for the following reasons.

  1. if the name is inconsistent.
  2. if the type is inconsistent.

Here's some considerations of adding this further check.
Pros:

  1. make the generated code size smaller.
  2. performance consideration.
    direct assignment is a reference copy, while in the serialization and deserialization, it's normally a deep copy which should probably be less efficient.

Cons:

  1. it will increase the codegen's implementation complexity and runtime complexity.
  2. it will probably be buggy if we miss consideration of a specific case.
  3. previously in hlc, we don't allow extra properties for input, with reference copy, we probably won't be able to do that.
  4. it may be problematic if customer modifies that reference after pass it to our code but before we send out the request.

See comment in #2759 (comment)

@qiaozha qiaozha added HRLC P1 priority 1 labels Oct 18, 2024
@qiaozha qiaozha self-assigned this Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HRLC P1 priority 1
Projects
None yet
Development

No branches or pull requests

1 participant