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

[SPE-2004] Support const and default attributes [Other languages] #202

Closed
TristanSpeakEasy opened this issue Aug 23, 2023 · 3 comments
Closed
Labels
ClientSDKGeneration Created by Linear-GitHub Sync CSS Created by Linear-GitHub Sync Medium priority Created by Linear-GitHub Sync

Comments

@TristanSpeakEasy
Copy link
Member

TristanSpeakEasy commented Aug 23, 2023

Allow values to be set without user input

From SyncLinear.com | SPE-2004

@TristanSpeakEasy TristanSpeakEasy added ClientSDKGeneration Created by Linear-GitHub Sync CSS Created by Linear-GitHub Sync Medium priority Created by Linear-GitHub Sync labels Aug 23, 2023
@fergusdixon
Copy link

Hi,

We have a discriminator (generated via FastAPI/Pydantic Literal')

It generates the following snippets in the OpenAPI spec, however the speakeasy cli fails with:
ERROR validation error: [line 280] discriminator mapping ref #/components/schemas/Distribution must have a property named style of type string or enum ERROR failed to generate SDKs for python ✖

      "Distribution": {
        "properties": {
          "style": {
            "const": "distribution",
            "title": "Style",
            "default": "distribution"
          }
        },
        "type": "object",
        "title": "Distribution",
        "description": "Pydantic class for when we want the distribution on a particular date."
      }
          "statistic": {
            "oneOf": [
              {
                "$ref": "#/components/schemas/Mean"
              },
              {
                "$ref": "#/components/schemas/Quantile"
              },
              {
                "$ref": "#/components/schemas/Distribution"
              }
            ],
            "title": "Statistic",
            "discriminator": {
              "propertyName": "style",
              "mapping": {
                "distribution": "#/components/schemas/Distribution",
                "mean": "#/components/schemas/Mean",
                "quantile": "#/components/schemas/Quantile"
              }
            }
          }

It seems it is failing on the missing "type" tag on "Distribution", however the swagger editor is able to determine it correctly.

Is this related to implying types from const/default attributes not being supported?

@TristanSpeakEasy
Copy link
Member Author

TristanSpeakEasy commented Oct 18, 2023

Hi @fergusdixon thanks for the issue report.

So looking through the schemas you provided above the issue you are seeing is because your style schema doesn't have a type attribute like type: string.

We rely on strong typing in your OpenAPI spec as much as possible to ensure we are generating the right types, so when a type isn't present we treat it as a generic any type which in this case fails our validation for discriminators because we are try to ensure they are strings.

While json-schema does allow for loose typing like this and assumptions on the type of the schema being made on heuristics around what other properties are present in the schema, we do little of that ourselves except where we can be pretty confident we have the type right, for example if you have a properties field or an items field in your schema we can be pretty sure its either an object or array you intended for the type of the schema. But for things like deducing the type is a string from const or default values that requires as to parse those values as json and try and deduce the type but that is error prone especially if the string values might contain a date or number for example.

So we just don't assume type in these situations and required it to be explicitly defined.

Hope that provides some context around why this is failing

@fergusdixon
Copy link

Hey, thanks for the clarification and definitely makes sense. Frustrating that the FastAPI/Pydantic Literal types don't include this. I think we can find a workaround using Field(json_schema_extra={"type": "string"} on the Literal fields to force that verbose typing

@TristanSpeakEasy TristanSpeakEasy closed this as not planned Won't fix, can't repro, duplicate, stale Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ClientSDKGeneration Created by Linear-GitHub Sync CSS Created by Linear-GitHub Sync Medium priority Created by Linear-GitHub Sync
Projects
None yet
Development

No branches or pull requests

2 participants