aj-foster / open-api-generator

Open API code generator for Elixir
MIT License
100 stars 13 forks source link

StabilityAi missing request types #26

Closed koenusz closed 6 months ago

koenusz commented 1 year ago

Thank you for this library.

I was playing around with it a bit to generate some client code form the stabilityAi api..

One thing i noticed was that there are missing fields in the request types. For instance in the "/v1/generation/{engine_id}/text-to-image".

The following code gets generated:

defmodule Stability.TextToImageRequestBody do
  @moduledoc """
  Provides struct and type for TextToImageRequestBody
  """

  @type t :: %__MODULE__{}

  defstruct []

  @doc false
  @spec __fields__(atom) :: keyword
  def __fields__(type \\ :t)

  def __fields__(:t) do
    []
  end
end

The message should conform to a different type of schema though (example from the openapi.json)

    "example": {
      "cfg_scale": 7,
      "clip_guidance_preset": "FAST_BLUE",
      "height": 512,
      "width": 512,
      "sampler": "K_DPM_2_ANCESTRAL",
      "samples": 1,
      "steps": 75,
      "text_prompts": [
        {
          "text": "A lighthouse on a cliff",
          "weight": 1
        }
      ]
    }

The schema is defined as follows:

     "TextToImageRequestBody": {
        "type": "object",
        "allOf": [
          {
            "type": "object",
            "properties": {
              "height": {
                "$ref": "#/components/schemas/DiffuseImageHeight"
              },
              "width": {
                "$ref": "#/components/schemas/DiffuseImageWidth"
              },
              "text_prompts": {
                "$ref": "#/components/schemas/TextPromptsForTextToImage"
              }
            },
            "required": [
              "text_prompts"
            ]
          },
          {
            "$ref": "#/components/schemas/GenerationRequestOptionalParams"
          }
        ]

The steps I followed to generate this result are:

mrluc commented 1 year ago

I can confirm this!

I wonder why it is. There are examples of GitHub and OpenAI clients -- @koenusz is there anything that jumps out to you about those API definitions, versus that of Stability? I'll take a look when I get a chance too! 😆

Aside, unrelated: the examples in the wild for how to write a client weren't useful to me -- the GitHub client is so extensive due to its plugin mechanism that it's quite non-obvious what to take from it as a client author; and the simplest-looking client, an OpenAI one, is written in a way that doesn't work with common {:array, {SomeStruct, :t}} response types. I had to make a client based on an OpenAI client lib with some additional handling that.

mrluc commented 1 year ago

@koenusz I suspect it's due to their API using more of OpenAPI's surface area -- heavy use of discriminator and top-level allOf in the StabilityAI's OpenAPI definitions.

Compare against these other API, which don't use discriminator and don't have any instances of top-level schema with allOf (only more deeply-nested, I think that's accurate):

Half-measures:

@aj-foster or dear reader: do we know if OpenAPI has a conformance test suite? Ideally one with a folder full of API definitions and inputs that that pass/validation errs that should be generated, etc.

mrluc commented 1 year ago

Actually for my current uses -- I might just go maps all the way and do my own API on top.

Ah -- also this library also fails to generate lots of modules. So between the missing modules and the ones with empty fields, and the fact that most folks will only be using a few endpoints/calls -- might go for maps all the way.

I poked around, and I'm wondering if we (folks like me) should be looking into gRPC for StabilityAI. I'm seeing that they generate their OpenAPI definition from their protocol buffers / grpc. Well, there are pure-elixir protocol buffer implementations ...

Well -- for now, I'm just doing my own thing, probably with Req. :)

koenusz commented 1 year ago

Missclick :)

@koenusz I suspect it's due to their API using more of OpenAPI's surface area -- heavy use of discriminator and top-level allOf in the StabilityAI's OpenAPI definitions.

I was suspecting this as well. I did a quick inspection of the code and there I got the impression these nodes are handled. Still they do not seem to show up in the types that use them.

We could rewrite them to not do that, maybe even come up with an automated transform ... boo.

I think this i not a good solution, In the end I think the library should be able to handle any valid openapi schema file. If it i not able to its better to roll your own custom solution.

Ah -- also this library also fails to generate lots of modules. So between the missing modules and the ones with empty fields, and the fact that most folks will only be using a few endpoints/calls -- might go for maps all the way.

Another option to use the generator as a starting point and manually ad the missing types. Not ideal but I guess its less work than defining everything from scratch :)

aj-foster commented 1 year ago

Hey folks, thanks for opening this! Great reproduction steps and investigation. I have a bit of a curveball to throw at you, and that is the upcoming release candidate (will be 0.1.0-rc.0). It's a pretty significant rewrite of a lot of the core logic, and while I can't remember addressing the allOf issue specifically, that code did change. You can see the current work on the aj/plugins branch.

I still need to finish up a few fixes and write a migration guide, but that should happen shortly. If you have the chance, would you mind testing with the new version and seeing how it performs?

Thanks!

koenusz commented 1 year ago

Thats great,

I will run the new branch to see how it handles the stabilityai openapi.json.

koenusz commented 1 year ago

@aj-foster a small recomendation for your readme.

use only: :dev, since I think after the code is generated the library is not used anymore.

def deps do
  [
    {:oapi_generator, "0.1.0-rc.0", only: :dev}
  ]
end
koenusz commented 1 year ago

I ran the generator like before and I have a few observations.

The issue with the request type does not seem to be solved, I still get an empty type.

it now seems to ignore my config:

config :oapi_generator,
  default: [
    base_location: "lib/schema",
    base_module: Stability
  ]

Before the code got generated into the schema folder I configured, right now everything appears in the root of the project. image

koenusz commented 1 year ago

I found the issue base_location is changed into location, this has not been updated in the readme..md

WRONG!

config :oapi_generator, default: [
  output: [
  base_location: "lib/example",
    base_module: Example
  ]
]

CORRECT

config :oapi_generator, default: [
  output: [
  location: "lib/example",
    base_module: Example
  ]
]
mrluc commented 1 year ago

@koenusz did making that change fix anything with the empty response types? And then do you notice fully missing modules as well?

@aj-foster super cool -- I'm running some errands this morning, but I'll probably be able to test it out today too. However, if you don't remember work on 'discriminator' specifically, I'd be surprised if the rc solves it, just because Stability/that method of gRPC-to-OpenAPI appears to rely heavily on 'discriminator,' in order to imitate inheritance in oop.

koenusz commented 1 year ago

@koenusz did making that change fix anything with the empty response types? And then do you notice fully missing modules as well?

Did not notice missing modules, but may be me not looking close enough.

aj-foster commented 1 year ago

Thanks for the suggestions! I just updated the README to include only: :dev, runtime: false and fix the location config.

aj-foster commented 1 year ago

FYI, I suspect the 0.1.0-rc.0 release is missing a lot of schemas in a non-deterministic way (due to changes in map ordering while processing operations). Working on that now.

aj-foster commented 1 year ago

Just a quick note: version 0.1.0-rc.3 has a lot of fixes since the first release candidate. I haven't tried StabilityAI's spec yet, but you'll probably have a better experience now!

aj-foster commented 9 months ago

FYI, #36 introduces support for allOf. 🙂

aj-foster commented 6 months ago

Hello again 👋🏼

Release 0.1.0-rc.4 has #36 mentioned above as well as some important changes for generating schemas for sub-fields. If you have a chance, please let me know how it all works for you!