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

Support encode int as string #2771

Merged
merged 15 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .chronus/changes/int-encoding-2024-7-14-14-9-2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-python"
---

Support encode int as string
5 changes: 3 additions & 2 deletions eng/pipelines/ci-template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,15 @@ steps:
condition: and(succeeded(), ${{ parameters.unitTest }})

- script: inv regenerate
displayName: "Regenerate Code"
displayName: "Regenerate Code(autorest)"
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
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@msyyc msyyc Aug 27, 2024

Choose a reason for hiding this comment

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

npm run regenerate
displayName: "Regenerate Code"
displayName: "Regenerate Code(typespec)"
workingDirectory: $(Build.SourcesDirectory)/autorest.python/packages/${{parameters.folderName}}/
condition: and(succeeded(), ${{ parameters.regenerate }}, eq('${{parameters.folderName}}', 'typespec-python'))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,12 @@ def default_template_representation_declaration(self) -> str:


class IntegerType(NumberType):

def __init__(self, yaml_data: Dict[str, Any], code_model: "CodeModel") -> None:
super().__init__(yaml_data=yaml_data, code_model=code_model)
if yaml_data.get("encode") == "string":
msyyc marked this conversation as resolved.
Show resolved Hide resolved
self.encode = "str"

@property
def serialization_type(self) -> str:
return "int"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,10 @@ def _serialize(o, format: typing.Optional[str] = None): # pylint: disable=too-m
return float(o)
if isinstance(o, enum.Enum):
return o.value
if isinstance(o, int):
if format == "str":
return str(o)
return o
try:
# First try datetime.datetime
return _serialize_datetime(o, format)
Expand Down Expand Up @@ -675,6 +679,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":
Copy link
Contributor

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 a deserialization function into the rest_field and have it serialize and deserialize as a string

Copy link
Member Author

Choose a reason for hiding this comment

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

class _RestField:
def __init__(
self,
*,
name: typing.Optional[str] = None,
type: typing.Optional[typing.Callable] = None, # pylint: disable=redefined-builtin
is_discriminator: bool = False,
visibility: typing.Optional[typing.List[str]] = None,
default: typing.Any = _UNSET,
format: typing.Optional[str] = None,
is_multipart_file_input: bool = False,
):

Appreciate if you could explain more how to pass in deserialization function? It seems that RestField has no parameter to pass it.

Copy link
Contributor

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 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

Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Contributor

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 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

Copy link
Member Author

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.

Copy link
Contributor

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 and deserialization, #2801

return int
return None

# is it a type alias?
Expand Down
4 changes: 2 additions & 2 deletions packages/typespec-python/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"@azure-tools/typespec-azure-resource-manager": "~0.45.0",
"@azure-tools/typespec-autorest": "~0.45.0",
"@azure-tools/cadl-ranch-expect": "~0.15.1",
"@azure-tools/cadl-ranch-specs": "~0.35.4",
"@azure-tools/cadl-ranch-specs": "~0.36.1",
"@types/js-yaml": "~4.0.5",
"@types/node": "^18.16.3",
"@types/yargs": "17.0.32",
Expand All @@ -77,7 +77,7 @@
"rimraf": "~5.0.0",
"typescript": "~5.1.3",
"@azure-tools/typespec-azure-core": "~0.45.0",
"@azure-tools/typespec-client-generator-core": "0.45.1",
"@azure-tools/typespec-client-generator-core": "0.45.4",
"@typespec/compiler": "~0.59.1",
"@typespec/http": "~0.59.0",
"@typespec/rest": "~0.59.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/typespec-python/scripts/eng/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "";
Copy link
Member Author

@msyyc msyyc Aug 23, 2024

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:
image

After #2775 merged, we could revert this change.

executeCommand(`npx eslint . --ext .ts ${checkWarning} `, "eslint");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ def _serialize(o, format: typing.Optional[str] = None): # pylint: disable=too-m
return float(o)
if isinstance(o, enum.Enum):
return o.value
if isinstance(o, int):
if format == "str":
return str(o)
return o
try:
# First try datetime.datetime
return _serialize_datetime(o, format)
Expand Down Expand Up @@ -660,6 +664,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":
return int
return None

# is it a type alias?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ def _serialize(o, format: typing.Optional[str] = None): # pylint: disable=too-m
return float(o)
if isinstance(o, enum.Enum):
return o.value
if isinstance(o, int):
if format == "str":
return str(o)
return o
try:
# First try datetime.datetime
return _serialize_datetime(o, format)
Expand Down Expand Up @@ -660,6 +664,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":
return int
return None

# is it a type alias?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ def _serialize(o, format: typing.Optional[str] = None): # pylint: disable=too-m
return float(o)
if isinstance(o, enum.Enum):
return o.value
if isinstance(o, int):
if format == "str":
return str(o)
return o
try:
# First try datetime.datetime
return _serialize_datetime(o, format)
Expand Down Expand Up @@ -660,6 +664,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":
return int
return None

# is it a type alias?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ def _serialize(o, format: typing.Optional[str] = None): # pylint: disable=too-m
return float(o)
if isinstance(o, enum.Enum):
return o.value
if isinstance(o, int):
if format == "str":
return str(o)
return o
try:
# First try datetime.datetime
return _serialize_datetime(o, format)
Expand Down Expand Up @@ -660,6 +664,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":
return int
return None

# is it a type alias?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ def _serialize(o, format: typing.Optional[str] = None): # pylint: disable=too-m
return float(o)
if isinstance(o, enum.Enum):
return o.value
if isinstance(o, int):
if format == "str":
return str(o)
return o
try:
# First try datetime.datetime
return _serialize_datetime(o, format)
Expand Down Expand Up @@ -660,6 +664,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":
return int
return None

# is it a type alias?
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
include *.md
include LICENSE
include azure/clientgenerator/core/flattenproperty/py.typed
recursive-include tests *.py
recursive-include samples *.py *.md
include azure/__init__.py
include azure/clientgenerator/__init__.py
include azure/clientgenerator/core/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@


# Azure Clientgenerator Core Flattenproperty client library for Python
<!-- write necessary description of service -->

## Getting started

### Install the package

```bash
python -m pip install azure-clientgenerator-core-flattenproperty
```

#### Prequisites

- Python 3.8 or later is required to use this package.
- You need an [Azure subscription][azure_sub] to use this package.
- An existing Azure Clientgenerator Core Flattenproperty instance.

## Contributing

This project welcomes contributions and suggestions. Most contributions require
you to agree to a Contributor License Agreement (CLA) declaring that you have
the right to, and actually do, grant us the rights to use your contribution.
For details, visit https://cla.microsoft.com.

When you submit a pull request, a CLA-bot will automatically determine whether
you need to provide a CLA and decorate the PR appropriately (e.g., label,
comment). Simply follow the instructions provided by the bot. You will only
need to do this once across all repos using our CLA.

This project has adopted the
[Microsoft Open Source Code of Conduct][code_of_conduct]. For more information,
see the Code of Conduct FAQ or contact opencode@microsoft.com with any
additional questions or comments.

<!-- LINKS -->
[code_of_conduct]: https://opensource.microsoft.com/codeofconduct/
[authenticate_with_token]: https://docs.microsoft.com/azure/cognitive-services/authentication?tabs=powershell#authenticate-with-an-authentication-token
[azure_identity_credentials]: https://github.com/Azure/azure-sdk-for-python/tree/main/sdk/identity/azure-identity#credentials
[azure_identity_pip]: https://pypi.org/project/azure-identity/
[default_azure_credential]: https://github.com/Azure/azure-sdk-for-python/tree/main/sdk/identity/azure-identity#defaultazurecredential
[pip]: https://pypi.org/project/pip/
[azure_sub]: https://azure.microsoft.com/free/

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"CrossLanguagePackageId": "Azure.ClientGenerator.Core.FlattenProperty",
"CrossLanguageDefinitionId": {
"azure.clientgenerator.core.flattenproperty.models.ChildFlattenModel": "Azure.ClientGenerator.Core.FlattenProperty.ChildFlattenModel",
"azure.clientgenerator.core.flattenproperty.models.ChildModel": "Azure.ClientGenerator.Core.FlattenProperty.ChildModel",
"azure.clientgenerator.core.flattenproperty.models.FlattenModel": "Azure.ClientGenerator.Core.FlattenProperty.FlattenModel",
"azure.clientgenerator.core.flattenproperty.models.NestedFlattenModel": "Azure.ClientGenerator.Core.FlattenProperty.NestedFlattenModel",
"azure.clientgenerator.core.flattenproperty.FlattenPropertyClient.put_flatten_model": "Azure.ClientGenerator.Core.FlattenProperty.putFlattenModel",
"azure.clientgenerator.core.flattenproperty.FlattenPropertyClient.put_nested_flatten_model": "Azure.ClientGenerator.Core.FlattenProperty.putNestedFlattenModel"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# coding=utf-8
# --------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
# Code generated by Microsoft (R) Python Code Generator.
# Changes may cause incorrect behavior and will be lost if the code is regenerated.
# --------------------------------------------------------------------------

from ._client import FlattenPropertyClient
from ._version import VERSION

__version__ = VERSION

try:
from ._patch import __all__ as _patch_all
from ._patch import * # pylint: disable=unused-wildcard-import
except ImportError:
_patch_all = []
from ._patch import patch_sdk as _patch_sdk

__all__ = [
"FlattenPropertyClient",
]
__all__.extend([p for p in _patch_all if p not in __all__])

_patch_sdk()
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
from azure.core.pipeline import policies
from azure.core.rest import HttpRequest, HttpResponse

from ._configuration import FlattenClientConfiguration
from ._operations import FlattenClientOperationsMixin
from ._configuration import FlattenPropertyClientConfiguration
from ._operations import FlattenPropertyClientOperationsMixin
from ._serialization import Deserializer, Serializer


class FlattenClient(FlattenClientOperationsMixin): # pylint: disable=client-accepts-api-version-keyword
class FlattenPropertyClient(FlattenPropertyClientOperationsMixin): # pylint: disable=client-accepts-api-version-keyword
"""Illustrates the model flatten cases.

:keyword endpoint: Service host. Default value is "http://localhost:3000".
Expand All @@ -30,7 +30,7 @@ def __init__( # pylint: disable=missing-client-constructor-parameter-credential
self, *, endpoint: str = "http://localhost:3000", **kwargs: Any
) -> None:
_endpoint = "{endpoint}"
self._config = FlattenClientConfiguration(endpoint=endpoint, **kwargs)
self._config = FlattenPropertyClientConfiguration(endpoint=endpoint, **kwargs)
_policies = kwargs.pop("policies", None)
if _policies is None:
_policies = [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# coding=utf-8
# --------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
# Code generated by Microsoft (R) Python Code Generator.
# Changes may cause incorrect behavior and will be lost if the code is regenerated.
# --------------------------------------------------------------------------

from typing import Any

from azure.core.pipeline import policies

from ._version import VERSION


class FlattenPropertyClientConfiguration: # pylint: disable=too-many-instance-attributes,name-too-long
"""Configuration for FlattenPropertyClient.

Note that all parameters used to create this instance are saved as instance
attributes.

:param endpoint: Service host. Default value is "http://localhost:3000".
:type endpoint: str
"""

def __init__(self, endpoint: str = "http://localhost:3000", **kwargs: Any) -> None:

self.endpoint = endpoint
kwargs.setdefault("sdk_moniker", "clientgenerator-core-flattenproperty/{}".format(VERSION))
self.polling_interval = kwargs.get("polling_interval", 30)
self._configure(**kwargs)

def _configure(self, **kwargs: Any) -> None:
self.user_agent_policy = kwargs.get("user_agent_policy") or policies.UserAgentPolicy(**kwargs)
self.headers_policy = kwargs.get("headers_policy") or policies.HeadersPolicy(**kwargs)
self.proxy_policy = kwargs.get("proxy_policy") or policies.ProxyPolicy(**kwargs)
self.logging_policy = kwargs.get("logging_policy") or policies.NetworkTraceLoggingPolicy(**kwargs)
self.http_logging_policy = kwargs.get("http_logging_policy") or policies.HttpLoggingPolicy(**kwargs)
self.custom_hook_policy = kwargs.get("custom_hook_policy") or policies.CustomHookPolicy(**kwargs)
self.redirect_policy = kwargs.get("redirect_policy") or policies.RedirectPolicy(**kwargs)
self.retry_policy = kwargs.get("retry_policy") or policies.RetryPolicy(**kwargs)
self.authentication_policy = kwargs.get("authentication_policy")
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ def _serialize(o, format: typing.Optional[str] = None): # pylint: disable=too-m
return float(o)
if isinstance(o, enum.Enum):
return o.value
if isinstance(o, int):
if format == "str":
return str(o)
return o
try:
# First try datetime.datetime
return _serialize_datetime(o, format)
Expand Down Expand Up @@ -660,6 +664,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":
return int
return None

# is it a type alias?
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# coding=utf-8
# --------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
# Code generated by Microsoft (R) Python Code Generator.
# Changes may cause incorrect behavior and will be lost if the code is regenerated.
# --------------------------------------------------------------------------

from ._operations import FlattenPropertyClientOperationsMixin

from ._patch import __all__ as _patch_all
from ._patch import * # pylint: disable=unused-wildcard-import
from ._patch import patch_sdk as _patch_sdk

__all__ = [
"FlattenPropertyClientOperationsMixin",
]
__all__.extend([p for p in _patch_all if p not in __all__])
_patch_sdk()
Loading
Loading