Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support encode int as string #2771
Support encode int as string #2771
Changes from 9 commits
443ad95
1181ccf
a4ccf83
208e6fd
5b60e60
342431e
5486ab2
c9771a4
6a98ad9
a41f973
8480487
8e20f21
f816fdb
cee1219
bc9dfb2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think we have to modify
model_base.py
, we can pass in adeserialization
function into therest_field
and have it serialize and deserialize as a stringThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
autorest.python/packages/typespec-python/generator/pygen/codegen/templates/model_base.py.jinja2
Lines 829 to 840 in 8480487
Appreciate if you could explain more how to pass in
deserialization
function? It seems that RestField has no parameter to pass it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you pass it through the
type
kwarg. For example, in this test, we've typed the prop as being a sequence, but in terms of serislizstion / deserialization, we want it to be aset
, so we pass in a callback. We should be able to do the same thing here, where we type it as an int, but we pass intype=lambda x: str(x)
/ write a callback that does that insteadThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for datetime/bytes encoding, we put the logic in
model_base.py
. shall we keep consistent for all the encoding things? another thing is if use callback, how to deal with serialization?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iscai-msft I think it is better not handle this feature with call_back so that we could keep consistent for all encoding things. But your comment hint me that we could confirm the deserialize and serialize function for model property when generate code so that we could save time for model initialization and I have created #2797 to track it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the model is vendored we can def change things, so I think we can have the rest field take in
serialization
anddeserialization
as callbacks. As @msyyc said we've already thought of passing explicit callbacks into rest field for perf improvements, so I think what we should do is do a slight redesign where we haveserialization
anddeserialization
callback inputs. When we're generating, we can generate with explicit callback inputs so we don't have to use perf time to determine which serialize / deserialize function we should useThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. And the change of redesign for
serialization
/deserialization
will be a little huge so I would like to do it in another PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I've added a separate issue for the
serialization
anddeserialization
, #2801There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
skip eslint failure caused by deprecation of
urlencode
:After #2775 merged, we could revert this change.