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

feat: generated python-sdk #94

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

feat: generated python-sdk #94

wants to merge 37 commits into from

Conversation

jarinox
Copy link
Collaborator

@jarinox jarinox commented Feb 15, 2024

  • To reduce development work on the python sdk it will be generated using swagger-codegen
  • This PR includes the generated sdk merged together with the old sdk where old classes and methods have been marked deprecated. This makes the new sdk available while still providing a copy of the old sdk.

@jarinox jarinox self-assigned this Feb 15, 2024
- sdk has been generated by swagger-codegen
- It is still possible to use the old sdk
- ORS_API_KEY secret must be set in settings for workflow to work
@MichaelsJP MichaelsJP marked this pull request as draft February 28, 2024 11:45
@MichaelsJP MichaelsJP marked this pull request as ready for review February 28, 2024 11:45
@koebi
Copy link
Collaborator

koebi commented Mar 5, 2024

Hey,

I played around with this for a bit, and out of the box, the following things are apparent:

  • until I have a client, I need 3 (from my perspective unintuitive) calls
  • I need to write my API key manually to a dict. typos here will lead to unclear errors.
  • I need a DirectionsService to request DirectionsServiceApi. This is both unclearly named and unintuitive
  • the return type of get_geo_json_route is an InlineResponse2003 which is unintuitively named and no valid json, though the name suggests that.
  • The module exports around 200 different classes, which makes it very confusing and obfuscated

The current module has 1 client class, 1 method/class per endpoint and a bit of util, so ~20 in total. That makes it rather clear to see what I want to use.

With this, the client might get easier to maintain, but currently, it certainly doesn't get easier to use.
To get any information, I still have to go through any sort of route['features'][0]['properties']['summary']['duration'], but the way to route is harder.

Regarding the PR structure:

  • Documentation is in this PR, although another for documentation exists?

@jarinox
Copy link
Collaborator Author

jarinox commented Mar 5, 2024

This PR contains the generated client and deprecation of the old client. The documentation contained in this PR is generated by swagger-codegen alongside the code.

The other PR contains vitepress configuration to host the generated docs.

@jarinox
Copy link
Collaborator Author

jarinox commented Mar 5, 2024

The generated client in fact requires more code to achieve the same things. This was one of the first things I noticed and discussed when experimenting with the codegen.

Beside less maintenance the generated classes allow cleaner code when performing complex requests as the parameters are not specified as map but rather defined as objects.

- Add snapping service
- Adapt MatrixServiceApi test to new client
- Remove unimplemented test templates
@jarinox jarinox changed the title Generated python-sdk feat: generated python-sdk Mar 19, 2024
@MichaelsJP MichaelsJP marked this pull request as draft March 19, 2024 16:09
@MichaelsJP MichaelsJP marked this pull request as ready for review March 19, 2024 16:09
@jarinox jarinox force-pushed the generated-client branch 3 times, most recently from 61565e7 to 88e7ca9 Compare March 26, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants