Skip to content

New Client Generator: Python uplink (+marshmallow) #2270

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

cognifloyd
Copy link
Contributor

PR checklist

  • Read the contribution guidelines.
  • [partial] Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh, ./bin/security/{LANG}-petstore.sh and ./bin/openapi3/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 3.4.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

WIP python-uplink client SDK generator

This client SDK generator uses the uplink and marshmallow libraries.
This began by forking the python client generator, discarding most of
the python code, and then re-writing the python portion.

uplink handles the communication with the server using user-pluggable
HTTP backends including requests, twisted, and aiohttp. The current
python-client generator requires selecting the library to use at
generation time, but uplink leaves such a choice up to the API SDK user
instead of the API SDK author.

marshmallow handles the model de/serialization to/from namedtuple
models, including validation at various points. Uplink comes with
support for marshmallow so it was easiest to just use that instead of
rolling a custom model library again.

The static auth method classes should, for the most part, be in the
upstream uplink project. Most of them have been merged into master, but
those changes have not made it into a release yet, so I included them
here (I wrote them based on a few other classes in uplink which is under
the MIT license).

Other marshmallow and uplink helper classes may also make their way
upstream.

TODO:

  • Add model generation support for composed schemas: All/Any/OneOf
  • Add a Marshmallow Bytes field (String is unicode)
  • Use Bytes field for types: file, binary, ByteArray, password
  • Make sure model generation works for all cases including when
    an array or other primitive is the top level type in a schema.
  • generate client lib tests (stubs?) that actually do something.
  • provide defaultValue for: Date, DateTime, BigDecimal
  • add option/switch(es) to allow serializing default values
    and/or deserializing missing values in client-side models
    (this generator leaves defaultValue inclusion to the API server)
  • Add first class cookie support to uplink
  • Add oauth support
  • Fix the generated docs so they're actually helpful/informative.
    (So far, only the example was updated).
  • Ensure that the partials loading works on windows (possibly
    by replaing / with \ when looking up partials on windows)

@wing328
Copy link
Member

wing328 commented Mar 1, 2019

Please replace tabs with 4-space for the following lines:

modules/openapi-generator/src/test/java/org/openapitools/codegen/options/PythonUplinkClientCodegenOptionsProvider.java:		        .put(PythonUplinkClientCodegen.PACKAGE_URL, PACKAGE_URL_VALUE)
modules/openapi-generator/src/test/java/org/openapitools/codegen/options/PythonUplinkClientCodegenOptionsProvider.java:		        .put(PythonUplinkClientCodegen.CLIENT_CLASS_NAME, CLIENT_CLASS_NAME)

@cognifloyd
Copy link
Contributor Author

I resolved the copyright and spacing issues. Does anything other than the Java files need a copyright notice?

This client SDK generator uses the uplink and marshmallow libraries.
This began by forking the python client generator, discarding most of
the python code, and then re-writing the python portion.

uplink handles the communication with the server using user-pluggable
HTTP backends including requests, twisted, and aiohttp. The current
python-client generator requires selecting the library to use at
generation time, but uplink leaves such a choice up to the API SDK user
instead of the API SDK author.

marshmallow handles the model de/serialization to/from namedtuple
models, including validation at various points. Uplink comes with
support for marshmallow so it was easiest to just use that instead of
rolling a custom model library again.

The static auth method classes should, for the most part, be in the
upstream uplink project. Most of them have been merged into master, but
those changes have not made it into a release yet, so I included them
here (I wrote them based on a few other classes in uplink which is under
the MIT license).

Other marshmallow and uplink helper classes may also make their way
upstream.

TODO:

- [ ] Add model generation support for composed schemas: All/Any/OneOf
- [ ] Add a Marshmallow Bytes field (String is unicode)
- [ ] Use Bytes field for types: file, binary, ByteArray, password
- [ ] Make sure model generation works for all cases including when
      an array or other primitive is the top level type in a schema.
- [ ] generate client lib tests (stubs?) that actually do something.
- [ ] provide defaultValue for: Date, DateTime, BigDecimal
- [ ] add option/switch(es) to allow serializing default values
      and/or deserializing missing values in client-side models
      (this generator leaves defaultValue inclusion to the API server)
- [ ] Add first class cookie support to uplink
- [ ] Add oauth support
- [ ] Fix the generated docs so they're actually helpful/informative.
      (So far, only the example was updated).
- [ ] Ensure that the partials loading works on windows (possibly
      by replaing / with \ when looking up partials on windows)
@wing328
Copy link
Member

wing328 commented Mar 12, 2019

I resolved the copyright and spacing issues. Does anything other than the Java files need a copyright notice?

Only the Java files need those.

@wing328
Copy link
Member

wing328 commented May 9, 2019

@cognifloyd thanks for the PR. When you've time, can we have a quick chat?

I'm reachable via https://gitter.im (ID: wing328) or Google Hangout (email address in my Github profile)

@wing328 wing328 modified the milestones: 4.0.0, 4.0.1 May 9, 2019
@wing328 wing328 modified the milestones: 4.0.1, 4.0.2 May 31, 2019
@cognifloyd
Copy link
Contributor Author

I have this almost working for my own basic openapi specs. The biggest issue I have (before I can start working on all the missing features listed in the PR TODO) is with the types.

I need to find a way to change how the imports and types work so that the api operation gets the marshmallow schema for returnType and for parameter types instead of raw python types. But the models still need marshmallowFieldTypes. The whole type system confuses me so I've had to step around it so far to get what is necessary into the templates, at least for the models.

@wing328 wing328 modified the milestones: 4.0.2, 4.0.3 Jun 20, 2019
@wing328 wing328 modified the milestones: 4.0.3, 4.1.0 Jul 9, 2019
@wing328 wing328 modified the milestones: 4.1.0, 4.1.1 Aug 9, 2019
@Fjolnir-Dvorak
Copy link
Contributor

Did you found a solution for the type issue? I found something this week which looks promising:

@wing328 wing328 modified the milestones: 4.1.1, 4.1.2 Aug 26, 2019
@wing328 wing328 modified the milestones: 4.1.2, 4.1.3 Sep 11, 2019
@wing328 wing328 modified the milestones: 4.1.3, 4.2.0 Oct 4, 2019
@wing328 wing328 removed this from the 4.2.0 milestone Oct 30, 2019
@michaelwiles
Copy link

What's the status of this? I am keen to get an uplink generator in that uses pydantic for the marshalling. I think it best if this is a separate generator...

@spacether
Copy link
Contributor

@cognifloyd do you still want this merged?
If so how about resolving the merge conflicts and we can move forward with it.
Sorry for the delay here, we need more python language reviewers.

@cognifloyd
Copy link
Contributor Author

Yes, but it'll be awhile before I have time to work on this.

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.

5 participants