tazatechnology / openapi_spec

Dart based OpenAPI specification generator and parser
BSD 3-Clause "New" or "Revised" License
8 stars 5 forks source link

Client generator streaming support #34

Closed walsha2 closed 10 months ago

walsha2 commented 10 months ago

Streaming support: if you check the implementation I'm subclassing the generated client to add streaming support manually. It may be possible to generate that code as well. The only piece that you cannot generate automatically from the spec is the StreamTransformer which could be provided as a ClientGeneratorOptions.

_Originally posted by @davidmigloz in https://github.com/tazatechnology/openapi_spec/issues/32#issuecomment-1791683173_

walsha2 commented 10 months ago

@davidmigloz funny you say that. I acutally did look at your implementation earlier and wondered why you subclassed, then I realized it was for the streaming part. Makes sense and I immediately thought about a way to add support for this because it did look like you ended up copying a lot of the request building logic in _requestStream. I think I have a solution for at least this first part (to reduce code duplication).

It may be possible to generate that code as well

Thoughts on how this would be possible? How does the spec provide insight as to which endpoints are able to be streamed? Take completions/ as an example:

/completions:
  post:
    operationId: createCompletion
    tags:
      - Completions
    summary: Creates a completion for the provided prompt and parameters.
    requestBody:
      required: true
      content:
        application/json:
          schema:
            $ref: "#/components/schemas/CreateCompletionRequest"
    responses:
      "200":
        description: OK
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/CreateCompletionResponse"

The only piece that you cannot generate automatically from the spec is the StreamTransformer which could be provided as a ClientGeneratorOptions

Any thoughts on what that interface would look like?

davidmigloz commented 10 months ago

@davidmigloz funny you say that. I acutally did look at your implementation earlier and wondered why you subclassed, then I realized it was for the streaming part. Makes sense and I immediately thought about a way to add support for this because it did look like you ended up copying a lot of the request building logic in _requestStream. I think I have a solution for at least this first part (to reduce code duplication).

I've refactored a bit the _request method in https://github.com/tazatechnology/openapi_spec/pull/38/files, now the subclass can easily add streaming support without duplicating any request code.

Thoughts on how this would be possible? How does the spec provide insight as to which endpoints are able to be streamed? Take completions/ as an example:

The official OpenAI spec doesn't provide any info to differentiate streaming vs non-streaming. But it could be if the response is defined as text/event-stream instead application/json.

What I had in my mind was something like this:

/completions:
  post:
    operationId: createCompletion
    tags:
      - Completions
    summary: Creates a completion for the provided prompt and parameters.
    requestBody:
      required: true
      content:
        application/json:
          schema:
            $ref: "#/components/schemas/CreateCompletionRequest"
    responses:
      "200":
        description: OK
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/CreateCompletionResponse"
  post:
    operationId: createCompletionStream
    tags:
      - Completions
    summary: Creates a completion for the provided prompt and parameters.
    requestBody:
      required: true
      content:
        application/json:
          schema:
            $ref: "#/components/schemas/CreateCompletionRequest"
    responses:
      "200":
        description: OK
        content:
          text/event-stream:
            schema:
              $ref: "#/components/schemas/CreateCompletionResponse"

However, I'm realising now that you cannot define the endpoint two times nor two posts for the same endpoint. So it is not feasible..

There doesn't seem to be any convention for SSE in the OpenAPI spec yet. So I guess the only option would be to use OpenAPI extensions (but it is something we want to avoid for this package as discussed).

Anyway, the amount of code required to support streaming in the subclass is quite minimal, so I'm not really annoyed by having to maintain it manually.

I think we can close this issue until the OpenAPI spec adds support for it.

walsha2 commented 10 months ago

Anyway, the amount of code required to support streaming in the subclass is quite minimal, so I'm not really annoyed by having to maintain it manually. I think we can close this issue until the OpenAPI spec adds support for it.

Sounds good! The refactor in #38 is exactly what I was thinking as well. Yea, your maintained manual code should be super minimal.