Skip to content

[Refactor] create Python abstract class#8817

Merged
wing328 merged 3 commits intomasterfrom
python-abstract-class
Feb 25, 2021
Merged

[Refactor] create Python abstract class#8817
wing328 merged 3 commits intomasterfrom
python-abstract-class

Conversation

@wing328
Copy link
Copy Markdown
Member

@wing328 wing328 commented Feb 24, 2021 •
edited
Loading

  • create Python abstract class
  • minor code format change (Java files)
  • only change in the samples (output) is the addition of title in the model's properties

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.
  • Run the following to build the project and update samples: 
    ./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
    Commit all changed files.
    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, 5.1.x, 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

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)

@wing328 wing328 added Client: Python Enhancement: Code Cleanup General refactoring, removal of deprecated things, commenting, etc. Server: Python Enhancement: Code format labels Feb 24, 2021
@wing328 wing328 added this to the 5.1.0 milestone Feb 24, 2021
@wing328 wing328 mentioned this pull request Feb 24, 2021
Closed
5 tasks
spacether
spacether reviewed Feb 25, 2021
View reviewed changes
Comment thread docs/generators/python-aiohttp.md
## RESERVED WORDS

<ul class="column-ul">
<li>all_params</li>
Copy link
Copy Markdown
Contributor

@spacether spacether Feb 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Why is all_params a reserved word?
The same question for

  • body_params
  • form_params
  • path_params
  • auth_settings
  • local_var_files
  • query_params
    Why not use an underscore prefix like _all_params to prevent name conflicts with params named all_params for all variable names that are not python reserved words?

Copy link
Copy Markdown
Member Author

@wing328 wing328 Feb 25, 2021

Choose a reason for hiding this comment

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

Noticed that too. Will file another PR to clean up the reserved word list.

spacether
spacether approved these changes Feb 25, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@spacether spacether left a comment

Choose a reason for hiding this comment

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

Looks good; some non-blocking questions.

@spacether
Copy link
Copy Markdown
Contributor

spacether commented Feb 25, 2021

@wing328 does this fix #8420?
Could you make this change to fix it?

By eye it looks like the error is happening because we add these files twice
If we stop adding it twice will this be fixed?

https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java#L130
openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonLegacyClientCodegen.java

Line 291 in 008ddfb

supportingFiles.add(new SupportingFile("__init__model.mustache", packagePath() + File.separatorCh

@wing328
Copy link
Copy Markdown
Member Author

wing328 commented Feb 25, 2021 •
edited
Loading

Don't think it will fix #8420. Will look into it and file a PR if I can come up with a fix.

@wing328 wing328 merged commit df1ae03 into master Feb 25, 2021
@wing328 wing328 deleted the python-abstract-class branch February 25, 2021 06:10
@Gidgidonihah Gidgidonihah mentioned this pull request May 21, 2021
Closed
@ohhai
Copy link
Copy Markdown

ohhai commented Apr 25, 2022

@wing328 could you please tell what is the current reason to map binary response type to string (not to byte array)?
I mean this comment: #4372 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@spacether spacether spacether approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Client: Python Enhancement: Code Cleanup General refactoring, removal of deprecated things, commenting, etc. Enhancement: Code format Server: Python

Projects

None yet

Milestone

5.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@wing328 @spacether @ohhai