Skip to content

Conversation

@s373r
Copy link
Member

@s373r s373r commented Apr 10, 2025

Description

Backported from #1188

  • This PR adds an automatically generated client (based on the OpenAPI (mt) schema) to interact with the REST API.
  • Generation using openapi-generator-cli with rust (client) generator:
    • The generator is used pretty much as is, except for changing the Cargo.toml template.
    • Added make codegen-kamu-api-client-mt target for the client generation.
    • ⚠️ Generation is manual only, there is no check on CI

Added

  • kamu-api-client-mt: autogenerated REST API client

Checklist before requesting a review

@s373r s373r self-assigned this Apr 10, 2025
@s373r s373r force-pushed the feature/opendatafabric-openapi-mt-client branch from 3c4d8c8 to 128e597 Compare April 10, 2025 09:45
@s373r s373r marked this pull request as ready for review April 10, 2025 09:53
Makefile Outdated


.PHONY: codegen-openapi-mt-client
codegen-openapi-mt-client:
Copy link
Member

@sergiimk sergiimk Apr 10, 2025

Choose a reason for hiding this comment

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

Oh my, I don't know how I feel about including such codegenned client crate...

Having it tied to CI, and using typed interfaces would bring us same benefits we are reaping from GQL codegen in Web UI already, so it probably makes sense. But I didn't think we would dedicate time to this at this stage.

If we were to adopt this there are a few things I'd like to see:

  1. Make sure it doesn't bring in new heavy depenencies, ie.:

    • It depents on same versions of e.g. reqwest and tokio that we are using and not pulls in a whole separate stack
      • I already see an issue that it pulls in openssl while the rest of the code is using rustls - this is a big no-no for me
  2. Let's get rid of auto-generated markdown docs - they just duplicate existing OpenAPI spec and don't bring any value.

  3. I don't think it belongs in odf layer as it uses endpoints from Kamu

Copy link
Member Author

Choose a reason for hiding this comment

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

Having it tied to CI, and using typed interfaces would bring us same benefits we are reaping from GQL codegen in Web UI already, so it probably makes sense.

To be honest, I've been eyeing autogeneration since the days of E2E test integration. In those, I manually make HTTP calls on the spot (cheap and fast).

Once again, when I needed to make an HTTP request to kamu-node (in #1188), I thought it's time to take a couple of hours to make a typed REST API client.

Make sure it doesn't bring in new heavy depenencies, ie.:

Revised the dependencies, the unnecessary ones are now unused.

Fixed

Let's get rid of auto-generated markdown docs - they just duplicate existing OpenAPI spec and don't bring any value.

Done

I don't think it belongs in odf layer as it uses endpoints from Kamu

Fair point -- moved it to src/utils/kamu-api-client-mt

Fixed

@s373r s373r changed the title opendatafabric-openapi-mt-client: autogenerated client for ODF REST API codegen-kamu-api-client-mt: autogenerated client for ODF REST API Apr 11, 2025
@s373r s373r requested a review from sergiimk April 11, 2025 10:54
@s373r s373r changed the title codegen-kamu-api-client-mt: autogenerated client for ODF REST API kamu-api-client-mt: autogenerated REST API client Apr 11, 2025
@zaychenko-sergei
Copy link
Contributor

zaychenko-sergei commented Apr 11, 2025

I guess we need it for API server more than for CLI.
In CLI it would likely benefit to e2e flows at most, while for API server there is a potential for public use.
For API server we might want to provide such auto-generated clients for several more languages, like Python, Java, Node.JS.

Another point, whether it should be a part of our workspace, or externalized?
Also, what would be the moment we invoke this auto-generation automatically, a release?

Should we have versioning, if we are externalizing this?

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