Support encode int as string #2771
Conversation
| @@ -50,7 +50,8 @@ export function pyright() { | |||
| } | |||
|
|
|||
| export function eslint() { | |||
| const checkWarning = argv.skipWarning ? "" : "--max-warnings=0"; | |||
| // const checkWarning = argv.skipWarning ? "" : "--max-warnings=0"; | |||
| const checkWarning = ""; | |||
There was a problem hiding this comment.
skip eslint failure caused by deprecation of urlencode:
After #2775 merged, we could revert this change.
packages/typespec-python/generator/pygen/codegen/models/primitive_types.py
Show resolved
Hide resolved
| @@ -675,6 +677,8 @@ def _get_deserialize_callable_from_annotation( # pylint: disable=R0911, R0915, | |||
| rf: typing.Optional["_RestField"] = None, | |||
| ) -> typing.Optional[typing.Callable[[typing.Any], typing.Any]]: | |||
| if not annotation or annotation in [int, float]: | |||
| if annotation is int and rf and rf._format == "str": | |||
There was a problem hiding this comment.
i don't think we have to modify model_base.py, we can pass in a deserialization function into the rest_field and have it serialize and deserialize as a string
There was a problem hiding this comment.
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.
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 a set, 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 in type=lambda x: str(x) / write a callback that does that instead
There was a problem hiding this comment.
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.
@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.
Since the model is vendored we can def change things, so I think we can have the rest field take in serialization and deserialization 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 have serialization and deserialization 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 use
There was a problem hiding this comment.
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.
Ok I've added a separate issue for the serialization and deserialization, #2801
| workingDirectory: $(Build.SourcesDirectory)/autorest.python/packages/${{parameters.folderName}}/ | ||
| condition: and(succeeded(), ${{ parameters.regenerate }}, eq('${{parameters.folderName}}', 'autorest.python')) | ||
|
|
||
| - script: | | ||
| find test/azure/generated -type f ! -name '*apiview_mapping_python.json*' -delete | ||
| find test/unbranded/generated -type f ! -name '*apiview_mapping_python.json*' -delete |
There was a problem hiding this comment.
why do we have this code here> we should already be doing this in the regenreate.ts file, and this is also for the autorest generation. Whatever functinoality we need from this should be moved into the scripts instead of being in yml for the scripts
There was a problem hiding this comment.
the one thing i'm not sure about is serialization. If we can't edo serialization here, we should instead add similar functionality but for serialization (and we can rename type to deserialization) instead of adding extra logic through the msrest way
There was a problem hiding this comment.
- there is condition to make sure it only works for typespec-python: https://github.com/Azure/autorest.python/blob/8e20f2136af3ae2e1939533a36c980b7c9bbe572/eng/pipelines/ci-template.yml#L121C12-L121C117
- If we add it in regenerate.ts, when regenreate fails, the local repo will be in a mess since all the generated folders are already deleted.
| @@ -675,6 +677,8 @@ def _get_deserialize_callable_from_annotation( # pylint: disable=R0911, R0915, | |||
| rf: typing.Optional["_RestField"] = None, | |||
| ) -> typing.Optional[typing.Callable[[typing.Any], typing.Any]]: | |||
| if not annotation or annotation in [int, float]: | |||
| if annotation is int and rf and rf._format == "str": | |||
There was a problem hiding this comment.
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 a set, 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 in type=lambda x: str(x) / write a callback that does that instead
fix #2703
pending on Azure/cadl-ranch#656