Skip to content

fix lro response setting #813

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

Merged
iscai-msft merged 7 commits into from
Nov 11, 2020
Merged

fix lro response setting #813

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
532d0f4
fix lro response setting
iscai-msft Nov 9, 2020
65d634f
update changelog
iscai-msft Nov 9, 2020
4db5d44
Merge branch 'autorestv3' of https://github.com/Azure/autorest.python…
iscai-msft Nov 10, 2020
e736363
switch check to check for num schemas
iscai-msft Nov 10, 2020
af5479f
address laurent's comments
iscai-msft Nov 10, 2020
f8bd9c1
Merge branch 'autorestv3' of https://github.com/Azure/autorest.python…
iscai-msft Nov 11, 2020
9fd1ac2
regen
iscai-msft Nov 11, 2020
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
10 changes: 10 additions & 0 deletions ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Change Log

### 2020-xx-xx - 5.4.2
Autorest core version: 3.0.6320

Modelerfour version: 4.15.421

**Bug Fixes**

- Correctly choose schema from response with 200 status code in the case of LRO operations with multiple responses #814


### 2020-11-03 - 5.4.1
Autorest core version: 3.0.6318

Expand Down
31 changes: 16 additions & 15 deletions autorest/codegen/models/lro_operation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,28 +58,29 @@ def __init__(
def set_lro_response_type(self) -> None:
if not self.responses:
return
responses = {response.schema: response for response in self.responses if response.has_body}
response_types = list(responses.values())
response_type = None
if len(response_types) > 1:
responses_with_bodies = [r for r in self.responses if r.has_body]
response = None
if len(responses_with_bodies) > 1:
Copy link
Contributor Author

@iscai-msft iscai-msft Nov 10, 2020

Choose a reason for hiding this comment

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

keep condition # response schemas

# choose the response that has a status code of 200
response_types_with_200_status_code = [
rt for rt in response_types if 200 in rt.status_codes
responses_with_200_status_codes = [
r for r in responses_with_bodies if 200 in r.status_codes
]
if not response_types_with_200_status_code:
try:
response = responses_with_200_status_codes[0]
schema_types = {r.schema for r in responses_with_bodies}
response_schema = cast(BaseSchema, response.schema).serialization_type
_LOGGER.warning(
"Multiple schema types in responses: %s. Choosing: %s", schema_types, response_schema
)
except IndexError:
raise ValueError(
f"Your swagger is invalid because you have multiple response schemas for LRO" +
f" method {self.python_name} and none of them have a 200 status code."
)
response_type = response_types_with_200_status_code[0]

response_type_schema_name = cast(BaseSchema, response_type.schema).serialization_type
_LOGGER.warning(
"Multiple schema types in responses: %s. Choosing: %s", response_types, response_type_schema_name
)
elif response_types:
response_type = response_types[0]
self.lro_response = response_type
elif responses_with_bodies:
response = responses_with_bodies[0]
self.lro_response = response

@property
def has_optional_return_type(self) -> bool:
Expand Down
6 changes: 1 addition & 5 deletions test/azure/Expected/AcceptanceTests/Lro/lro/aio/operations/_lro_retrys_operations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,14 +348,10 @@ async def begin_delete_provisioning202_accepted200_succeeded(
kwargs.pop('content_type', None)

def get_long_running_output(pipeline_response):
response_headers = {}
response = pipeline_response.http_response
response_headers['Location']=self._deserialize('str', response.headers.get('Location'))
response_headers['Retry-After']=self._deserialize('int', response.headers.get('Retry-After'))
deserialized = self._deserialize('Product', pipeline_response)

if cls:
return cls(pipeline_response, deserialized, response_headers)
return cls(pipeline_response, deserialized, {})
return deserialized

if polling is True: polling_method = AsyncARMPolling(lro_delay, **kwargs)
Expand Down
18 changes: 3 additions & 15 deletions test/azure/Expected/AcceptanceTests/Lro/lro/aio/operations/_lros_operations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2174,14 +2174,10 @@ async def begin_delete_provisioning202_accepted200_succeeded(
kwargs.pop('content_type', None)

def get_long_running_output(pipeline_response):
response_headers = {}
Copy link
Contributor Author

@iscai-msft iscai-msft Nov 9, 2020

Choose a reason for hiding this comment

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

I checked out these missing response headers, and I think we were incorrectly generating them in the first place. Basically what happened was that we were setting the 202 response as the final deserialization, when it should've been the 200 response

Copy link
Contributor Author

@iscai-msft iscai-msft Nov 9, 2020

Choose a reason for hiding this comment

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

I've checked all of the response headers where we no longer generate, and this is the case, @lmazuel what're your thoughts? I don't think this is a breaking change since it's really a bug fix, and all of the response headers we exposes before were empty

response = pipeline_response.http_response
response_headers['Location']=self._deserialize('str', response.headers.get('Location'))
response_headers['Retry-After']=self._deserialize('int', response.headers.get('Retry-After'))
deserialized = self._deserialize('Product', pipeline_response)

if cls:
return cls(pipeline_response, deserialized, response_headers)
return cls(pipeline_response, deserialized, {})
return deserialized

if polling is True: polling_method = AsyncARMPolling(lro_delay, **kwargs)
Expand Down Expand Up @@ -2278,14 +2274,10 @@ async def begin_delete_provisioning202_deleting_failed200(
kwargs.pop('content_type', None)

def get_long_running_output(pipeline_response):
response_headers = {}
response = pipeline_response.http_response
response_headers['Location']=self._deserialize('str', response.headers.get('Location'))
response_headers['Retry-After']=self._deserialize('int', response.headers.get('Retry-After'))
deserialized = self._deserialize('Product', pipeline_response)

if cls:
return cls(pipeline_response, deserialized, response_headers)
return cls(pipeline_response, deserialized, {})
return deserialized

if polling is True: polling_method = AsyncARMPolling(lro_delay, **kwargs)
Expand Down Expand Up @@ -2382,14 +2374,10 @@ async def begin_delete_provisioning202_deletingcanceled200(
kwargs.pop('content_type', None)

def get_long_running_output(pipeline_response):
response_headers = {}
response = pipeline_response.http_response
response_headers['Location']=self._deserialize('str', response.headers.get('Location'))
response_headers['Retry-After']=self._deserialize('int', response.headers.get('Retry-After'))
deserialized = self._deserialize('Product', pipeline_response)

if cls:
return cls(pipeline_response, deserialized, response_headers)
return cls(pipeline_response, deserialized, {})
return deserialized

if polling is True: polling_method = AsyncARMPolling(lro_delay, **kwargs)
Expand Down
6 changes: 1 addition & 5 deletions test/azure/Expected/AcceptanceTests/Lro/lro/operations/_lro_retrys_operations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,14 +358,10 @@ def begin_delete_provisioning202_accepted200_succeeded(
kwargs.pop('content_type', None)

def get_long_running_output(pipeline_response):
response_headers = {}
response = pipeline_response.http_response
response_headers['Location']=self._deserialize('str', response.headers.get('Location'))
response_headers['Retry-After']=self._deserialize('int', response.headers.get('Retry-After'))
deserialized = self._deserialize('Product', pipeline_response)

if cls:
return cls(pipeline_response, deserialized, response_headers)
return cls(pipeline_response, deserialized, {})
return deserialized

if polling is True: polling_method = ARMPolling(lro_delay, **kwargs)
Expand Down
18 changes: 3 additions & 15 deletions test/azure/Expected/AcceptanceTests/Lro/lro/operations/_lros_operations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2218,14 +2218,10 @@ def begin_delete_provisioning202_accepted200_succeeded(
kwargs.pop('content_type', None)

def get_long_running_output(pipeline_response):
response_headers = {}
response = pipeline_response.http_response
response_headers['Location']=self._deserialize('str', response.headers.get('Location'))
response_headers['Retry-After']=self._deserialize('int', response.headers.get('Retry-After'))
deserialized = self._deserialize('Product', pipeline_response)

if cls:
return cls(pipeline_response, deserialized, response_headers)
return cls(pipeline_response, deserialized, {})
return deserialized

if polling is True: polling_method = ARMPolling(lro_delay, **kwargs)
Expand Down Expand Up @@ -2324,14 +2320,10 @@ def begin_delete_provisioning202_deleting_failed200(
kwargs.pop('content_type', None)

def get_long_running_output(pipeline_response):
response_headers = {}
response = pipeline_response.http_response
response_headers['Location']=self._deserialize('str', response.headers.get('Location'))
response_headers['Retry-After']=self._deserialize('int', response.headers.get('Retry-After'))
deserialized = self._deserialize('Product', pipeline_response)

if cls:
return cls(pipeline_response, deserialized, response_headers)
return cls(pipeline_response, deserialized, {})
return deserialized

if polling is True: polling_method = ARMPolling(lro_delay, **kwargs)
Expand Down Expand Up @@ -2430,14 +2422,10 @@ def begin_delete_provisioning202_deletingcanceled200(
kwargs.pop('content_type', None)

def get_long_running_output(pipeline_response):
response_headers = {}
response = pipeline_response.http_response
response_headers['Location']=self._deserialize('str', response.headers.get('Location'))
response_headers['Retry-After']=self._deserialize('int', response.headers.get('Retry-After'))
deserialized = self._deserialize('Product', pipeline_response)

if cls:
return cls(pipeline_response, deserialized, response_headers)
return cls(pipeline_response, deserialized, {})
return deserialized

if polling is True: polling_method = ARMPolling(lro_delay, **kwargs)
Expand Down