[Python] correct return types if multiple responses are defined by ksvirkou-hubspot · Pull Request #7427 · OpenAPITools/openapi-generator

ksvirkou-hubspot · 2020-09-15T14:56:14Z

PR checklist

Read the contribution guidelines.
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
If contributing template-only or documentation-only changes which will change sample output, build the project beforehand.
Run the shell script ./bin/generate-samples.shto update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. For Windows users, please run the script in Git BASH.
File the PR against the correct branch: master
Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR
Fixed issue #7426
Also fixes #440

wing328 · 2020-09-17T10:40:33Z

cc @taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11) @tomplus (2018/10) @Jyhess (2019/01) @arun-nalla (2019/11) @spacether (2019/11)

spacether · 2020-09-17T15:13:36Z

modules/openapi-generator/src/main/resources/python/api.mustache

            post_params=form_params,
            files=local_var_files,
            response_type={{#returnType}}'{{returnType}}'{{/returnType}}{{^returnType}}None{{/returnType}},  # noqa: E501
+            response_types_map=response_types_map,


Rather than using two parameters for return type

return_type

response_types_map

how about using a single parameter like:

return_types which is a map of status code to data type

Please see this similar-to PR which was never finished

@spacether
I ve just removed response_type
Could you check the pr one more time ?

Thank you for using only one parameter. This pr has failing ci testa because the samples need to be regenerated. Can you please regenerate samples with these commands?

mvn clean install
bin/generate_samples

@spacether
I ve just regenerated

spacether · 2020-09-25T15:38:34Z

I am seeing an uncommitted changes error in Circle. Closing and re opening to kick off CI again.
If that does not fix it this needs a rebase on master then mvn clean install and bin/generate_samples

spacether · 2020-09-26T08:44:10Z

@ksvirkou-hubspot thank you for this PR. We are so close!
Can you:

rebase you branch on master?
mvn clean install
bin/generate_samples

…rator into python/multiple_responses

spacether · 2020-09-29T15:39:36Z

samples/client/petstore/python-asyncio/petstore_api/api/pet_api.py

+
+        response_types_map={
+            200: "Pet",
+            400: "",


Should these empty mappings be omitted?

Yes it Should be empty

"400": { "description": "Invalid status value" }

https://petstore.swagger.io/v2/swagger.json

So then should the value be None for these cases? It looks like that is what we were defaulting to before.
Adding a test that returns a response with an invalid status would verify that this does what you want it to do.
If you want to add that, here is a similar to test that tests sending to and receiving from a server and deserializing.
In it we mock the returned response data and we deserialize into whatever is defined in response_type/response_types_map.

@spacether
done

ksvirkou-hubspot · 2020-09-29T15:58:58Z

@ksvirkou-hubspot thank you for this PR. We are so close!
Can you:

rebase you branch on master?

mvn clean install

bin/generate_samples

ksvirkou-hubspot · 2020-09-29T15:59:55Z

@ksvirkou-hubspot thank you for this PR. We are so close!
Can you:

rebase you branch on master?

mvn clean install

bin/generate_samples

@spacether
done

spacether

Thank you for the PR. This looks great!
Labeling this as a breaking change with fallback because we are changing the call_api interface

update python mustache templates

7635651

ksvirkou-hubspot mentioned this pull request Sep 15, 2020

[Python] Incorrect return types if multiple responses are defined #7426

Closed

bug fix

fee3d88

This was linked to issues Sep 17, 2020

[Python] Incorrect return types if multiple responses are defined #7426

Closed

[REQ][Typescript-Node] Replace request library with node http request #7440

Open

spacether removed a link to an issue Sep 17, 2020

[REQ][Typescript-Node] Replace request library with node http request #7440

Open

spacether reviewed Sep 17, 2020

View reviewed changes

ksvirkou-hubspot added 3 commits September 21, 2020 18:15

rm response_type

9ea1c98

generated samples

46c52c6

resolve conflict

e3ad7df

spacether added the Client: Python label Sep 25, 2020

spacether added this to the 5.0.0 milestone Sep 25, 2020

spacether closed this Sep 25, 2020

spacether reopened this Sep 25, 2020

Merge branch 'master' of https://github.com/OpenAPITools/openapi-gene…

356536c

…rator into python/multiple_responses

spacether reviewed Sep 29, 2020

View reviewed changes

samples

4398efc

ksvirkou-hubspot closed this Sep 29, 2020

ksvirkou-hubspot reopened this Sep 29, 2020

ksvirkou-hubspot added 2 commits September 30, 2020 16:10

set None if a type is empty

97eaeca

regenerated samples

14e6a52

ksvirkou-hubspot requested a review from spacether September 30, 2020 19:55

spacether approved these changes Oct 3, 2020

View reviewed changes

spacether added the Breaking change (with fallback) label Oct 3, 2020

spacether merged commit 4e5ecf2 into OpenAPITools:master Oct 3, 2020

ksvirkou-hubspot deleted the python/multiple_responses branch October 15, 2020 13:33

ymilki mentioned this pull request Dec 23, 2021

[Python] Client code is wrong when an endpoint accepts multiple MediaTypes #440

Closed

Uh oh!

Conversation

ksvirkou-hubspot commented Sep 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

wing328 commented Sep 17, 2020

Uh oh!

spacether Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ksvirkou-hubspot Sep 21, 2020

Choose a reason for hiding this comment

Uh oh!

spacether Sep 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ksvirkou-hubspot Sep 23, 2020

Choose a reason for hiding this comment

Uh oh!

spacether commented Sep 25, 2020

Uh oh!

spacether commented Sep 26, 2020

Uh oh!

spacether Sep 29, 2020

Choose a reason for hiding this comment

Uh oh!

ksvirkou-hubspot Sep 29, 2020

Choose a reason for hiding this comment

Uh oh!

spacether Sep 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ksvirkou-hubspot Sep 30, 2020

Choose a reason for hiding this comment

Uh oh!

ksvirkou-hubspot commented Sep 29, 2020

Uh oh!

ksvirkou-hubspot commented Sep 29, 2020

Uh oh!

spacether left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ksvirkou-hubspot commented Sep 15, 2020 •

edited

Loading

spacether Sep 17, 2020 •

edited

Loading

spacether Sep 21, 2020 •

edited

Loading

spacether Sep 29, 2020 •

edited

Loading