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.

Sign up for GitHub

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

Jump to bottom

don't reformat query params for dpg next link calls #1168

Merged
merged 10 commits into from
Mar 3, 2022
Merged

don't reformat query params for dpg next link calls #1168

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
16dd817
don't reformat query params for dpg next link calls
iscai-msft Feb 28, 2022
5d4583b
add acceptance tests
iscai-msft Feb 28, 2022
925e002
undo test class
iscai-msft Feb 28, 2022
395db75
update changelog and version
iscai-msft Feb 28, 2022
30d5a0b
Merge branch 'autorestv3' into next_link_opaque
iscai-msft Mar 1, 2022
55ee823
Merge branch 'autorestv3' of https://github.com/Azure/autorest.python…
iscai-msft Mar 3, 2022
d9e2f0d
add flag reformat-next-link
iscai-msft Mar 3, 2022
478fc44
Merge branch 'next_link_opaque' of https://github.com/Azure/autorest.…
iscai-msft Mar 3, 2022
794ead2
have legacy calls do opaque next calls
iscai-msft Mar 3, 2022
56784c7
make next link paging part of 5.13.0
iscai-msft Mar 3, 2022
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
6 changes: 5 additions & 1 deletion ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Change Log

### 2022-xx-xx - 5.12.7
### 2022-xx-xx - 5.13.0

| Library | Min Version
| --------------- | -------
Expand All @@ -10,6 +10,10 @@
|`msrest` dep of generated code | `0.6.21`
|`azure-mgmt-core` dep of generated code (If generating mgmt plane code) | `1.3.0`

**Breaking Changes in Version Tolerant Generation**

- Version tolerant paging does not reformat initial query parameters into the next link #1168

**Bug Fixes**

- Add default value consistently for parameters #1164
Expand Down
3 changes: 2 additions & 1 deletion autorest/codegen/models/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from .imports import FileImport, ImportType, TypingSection
from .lro_operation import LROOperation
from .paging_operation import PagingOperation
from .parameter import Parameter, ParameterStyle
from .parameter import Parameter, ParameterStyle, ParameterLocation
from .operation import Operation
from .property import Property
from .operation_group import OperationGroup
Expand Down Expand Up @@ -50,6 +50,7 @@
"PagingOperation",
"Parameter",
"ParameterList",
"ParameterLocation",
"OperationGroup",
"Property",
"RequestBuilder",
Expand Down
13 changes: 13 additions & 0 deletions autorest/codegen/serializers/builder_serializer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
SchemaResponse,
IOSchema,
ParameterStyle,
ParameterLocation
)
from . import utils

Expand Down Expand Up @@ -798,6 +799,7 @@ def _call_request_builder_helper(
builder,
request_builder: RequestBuilder,
template_url: Optional[str] = None,
is_next_request: bool = False,
) -> List[str]:
retval = []
if len(builder.body_kwargs_to_pass_to_request_builder) > 1:
Expand Down Expand Up @@ -837,6 +839,15 @@ def _call_request_builder_helper(
parameter.serialized_name not in builder.body_kwargs_to_pass_to_request_builder
):
continue
if (
is_next_request and
not bool(builder.next_request_builder) and
self.code_model.options["version_tolerant"] and
Copy link
Member

Choose a reason for hiding this comment

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

#1165 is found in azure-mgmt-recoveryservicesbackup, so I think we shall remove the limitation version_tolerant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we ended up patching legacy code. The issue is it's a breaking change to change how we deal with next link calls, so we can't enable this for legacy, even if it might help one team (because other teams might be broken by it). But yeah, definitely good point because there is a legacy team affected by it

Copy link
Member

Choose a reason for hiding this comment

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

If we are afraid to break current legacy code, how about adding flag like --disable-filter-query-params to permit some service like recoveryservicesbackup to avoid duplicated filter params in legacy code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added flag --reformat-next-link, which defaults to True for legacy code generations, is forced to False for DPG generations. Thanks!

parameter.location == ParameterLocation.Query
):
# for version tolerant, we don't want to reformat query parameters if
# there is just one defined paging operation in the swagger
continue
high_level_name = cast(RequestBuilderParameter, parameter).name_in_high_level_operation
retval.append(f" {parameter.serialized_name}={high_level_name},")
if not self.code_model.options["version_tolerant"]:
Expand Down Expand Up @@ -1083,11 +1094,13 @@ def call_next_link_request_builder(self, builder) -> List[str]:
else:
request_builder = builder.request_builder
template_url = "next_link"

request_builder = builder.next_request_builder or builder.request_builder
return self._call_request_builder_helper(
builder,
request_builder,
template_url=template_url,
is_next_request=True
)

def _prepare_request_callback(self, builder) -> List[str]:
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@autorest/python",
"version": "5.12.6",
"version": "5.13.0",
"description": "The Python extension for generators in AutoRest.",
"scripts": {
"prepare": "node run-python3.js prepare.py",
Expand Down
Loading