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.

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

Adds memoization and deserialization through 2 or more discriminators #6124

Conversation

spacether
Copy link
Contributor

@spacether spacether commented Apr 30, 2020

Adds memoization with the decorator cached_property and deserialization through 2 or more discriminators in python-experimental
Memoization should decrease the deserialization time through complex schemas
The discriminator logic picking a different model to instantiate has been moved to the __new__ method.

  • tests have been added demonstrating the discriminator deserialization through 2 discriminators

@ethan92429 this may solve the problem you were having where deserialization takes a while

This PR along these:

Should resolve #4912

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.

Python Technical committee:
@taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11) @tomplus (2018/10) @Jyhess (2019/01) @slash-arun (2019/11) @spacether (2019/11)

@sebastien-rosset

This PR: #5809 also touches python-experimental discriminator code and is in review

@sebastien-rosset
Copy link
Contributor

Memoization should increase the deserialization time through complexschemas

Did you mean "decrease"?

@spacether
Copy link
Contributor Author

spacether commented May 1, 2020

@sebastien-rosset any tips on how to get the go-experimental code working?
I am seeing these errors in CI:
https://circleci.com/gh/OpenAPITools/openapi-generator/15047#config/containers/1
But those interfaces are not required in python-experimental because we iterate over:

{{#requiredVars}}
            '{{name}}': ({{{dataType}}},),  # noqa: E501
{{/requiredVars}}
{{#optionalVars}}
            '{{name}}': ({{{dataType}}},),  # noqa: E501
{{/optionalVars}}

in the composed models Shape/Triangle/Quadrilateral

The go-experimental template is assuming that oneOf child models contain the full interfaces at the composed model level.
So model_shape.go is assuming that model_triangle.go implements the GetShapeType interface.
In python we implement these interfaces with the {{#requiredVars}}.
How should I move forward here?

  • Separate off the spec for python-experimental only?

@spacether
Copy link
Contributor Author

spacether commented May 3, 2020

Adding ShapeInterface to Triangle and Quadrilateral is not possible because when we make that change, the codegenModel then drops the oneOf schemas due to this bug: #6143
so I will make a new python-experimental document for our v3 samples.

@spacether spacether merged commit cbc006a into OpenAPITools:master May 3, 2020
@spacether spacether deleted the adds_memoization_and_discriminator_traversal branch May 3, 2020 18:44
@wing328
Copy link
Member

wing328 commented May 7, 2020

@spacether thanks for the PR, which has been included in the v4.3.1 release: https://twitter.com/oas_generator/status/1258057660974329861

michaelpro1 pushed a commit to michaelpro1/openapi-generator that referenced this pull request May 7, 2020
…OpenAPITools#6124)

* Adds cached_property decorator, adds feature to move through n discriminators

* Adds v3 sample with 2 discriminators

* Adds tests of Shape, Tringle, and Quadrilateral traveling through 2 discriminators

* Adds test_deserialization.py:test_deserialize_shape

* Simplifes XTriangle sample spec schemas

* Simplifies Shape schema definition, updates go+python-experimental samples

* Fixes python-experimental test_dog tests

* Separates off python-experimental spec
jimschubert added a commit that referenced this pull request May 8, 2020
…-5.0

* origin/master: (78 commits)
  [powershell-experimental] : http signature authentication implementation (#6176)
  Add missing AnyType type mapping (#6196)
  remove pubspec.lock (#6208)
  Minor fixes post-release (#6204)
  [cpp][Qt5] Add the ability to pass QNetworkAccessManager as a parameter  (#6053)
  comment out dart2 test due to failure
  adds the missing typeMapping for AnyType (#6199)
  [Rust Server] Support boolean headers, and fix panic handling headers (#6056)
  update samples
  update 5.0.0 release date
  update readme with new release
  Prepare 4.3.1 release (#6187)
  Fix #6157: Updated native template to fix null async return (#6168)
  Show description when summary is missing (#6159)
  Make the array items optional (#6132)
  [aspnetcore] test petstore samples in drone.io (#6148)
  fix bearer auth in c# netcore async call (#6136)
  skip web.config for aspnetcore 3.x (#6147)
  Adds memoization and deserialization through 2 or more discriminators (#6124)
  Implement Asp.Net Core 3.0/3.1 generator (#6009) (#6025)
  ...
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.

[Python] Multilevel inheritance discriminator support
3 participants