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

Client codegen is overly restrictive regarding sync vs async, deserialized response type #11

Closed
Arnavion opened this issue Jun 22, 2018 · 0 comments

Comments

@Arnavion
Copy link
Owner

Arnavion commented Jun 22, 2018

The codegen knows:

  1. the HTTP method
  2. the URL (including parameters)
  3. the request body, if any
  4. the type to deserialize the response, specifically how watch/watchlist can handle infinite response bodies

The codegen should not have to know:

  1. the type of the client used to execute the request (reqwest, hyper, etc)
  2. whether the request is executed sync or async
  3. whether the request is executed directly without going through the API, such as when making a DELETE request to a pod's selfLink.

The current codegen expects any client that impls ::Client. But it also deserializes the response internally, so it has to require the request to be executed synchronously.

It also doesn't give the user the ability to override how the response is deserialized. This is especially important given issues like #8 and #9 where the API is unusable until the code generator patches mistakes in the upstream spec, or #10 which are bugs in the codegen itself. Ideally the process of creating a request with the URL and parameters should be separate from executing the request and parsing the response.


Alternatives:

  1. Begin/end-style API pairs with Client

    foo_begin<C: Client>(&C, &Args) -> C::Request + foo_end<C: Client>(C::Response) -> Result<FooResponse, C::Error>

    • C::Response still has to be std::io::Read so this limits asynchronous clients. An asynchronous client could not naively concat2()'d its response stream because that wouldn't work for watch/watchlist.
  2. Begin/end-style API pairs with http types

    foo_begin(&Args) -> http::Request + foo_end(http::Response<Vec<u8>>) -> Result<FooResponse>

    • The response still has to be http::Response<Vec<u8>> so that it can be std::io::Read, so this limits asynchronous clients in the same way.
  3. Only request API with http types. Deserializing response is user's responsibility.

    foo(&Args) -> http::Request + user deserializes response body to FooResponse

    • A bunch of match response.status_code() { OK => } boilerplate and hunting for the type to deserialize to (in the usual case where the codegen is correct) is pushed down to the user.

    See https://github.com/Arnavion/k8s-openapi-codegen/tree/sans-io branch for an implementation of this idea.

  4. Something else?

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

No branches or pull requests

1 participant