Skip to content

[Python] Additional properties with array types#6594

Merged
spacether merged 8 commits intoOpenAPITools:masterfrom
jirikuncar:python-experimental/additional-properties-array-enum
Jun 13, 2020
Merged

[Python] Additional properties with array types#6594
spacether merged 8 commits intoOpenAPITools:masterfrom
jirikuncar:python-experimental/additional-properties-array-enum

Conversation

@jirikuncar
Copy link
Contributor

@jirikuncar jirikuncar commented Jun 9, 2020

Describes issue with additionalProperties with array types.

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@jirikuncar
Copy link
Contributor Author

Can you please review this fix @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)?

<tr><td> 200 </td><td> Got object with additional properties with array of enums </td><td> - </td></tr>
</table>
*/
public Map<String, List<EnumClass>> getAdditionalPropertiesWithArrayOfEnums() throws ApiException {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like Java is suffering the same issue with missing import and not using named class for mapping.

@bbdouglas (2017/07) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) @karismann (2019/03) @Zomzog (2019/04) @lwlee2608 (2019/10) @bkabrda (2020/01)

Copy link
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.

Thank you for the PR. The python changes look good.
@jirikuncar It looks like the java changes are causing CI errors, happy to wait for fixes or break out the java work as a separate PR.
What do you want to do here?

@jirikuncar
Copy link
Contributor Author

@spacether I would prefer to have the Java fix in separate PR.

@spacether
Copy link
Contributor

@spacether I would prefer to have the Java fix in separate PR.

That sounds great. Can you extract the Java update into a separate PR and we can move forward with merging this fix in for python-experimental here?

@jirikuncar
Copy link
Contributor Author

@spacether I have changed only python related files. Why is the Java generated code changed?

@spacether
Copy link
Contributor

spacether commented Jun 11, 2020

@spacether I have changed only python related files. Why is the Java generated code changed?

Hmm it looks like it is because the yaml file is also used by jersey-2 :(
https://github.com/OpenAPITools/openapi-generator/blob/master/bin/configs/java-jersey2-8.yaml
So we either fix Java here, or make a separate version of the spec that jersey-2 uses which lacks an array model containing enums.

@jirikuncar
Copy link
Contributor Author

@spacether @jimschubert I have created a separate specification for Java.

@jirikuncar
Copy link
Contributor Author

@spacether -label:"Client: Java"

Copy link
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. Thank you for the PR!

@spacether spacether merged commit e07f084 into OpenAPITools:master Jun 13, 2020
@jirikuncar jirikuncar deleted the python-experimental/additional-properties-array-enum branch June 14, 2020 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants