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

OpenAPI 3 #558

Merged
merged 6 commits into from
Jan 23, 2024
Merged

OpenAPI 3 #558

merged 6 commits into from
Jan 23, 2024

Conversation

roberth
Copy link
Member

@roberth roberth commented Jan 7, 2024

Adds OpenAPI 3 schema in parallel to existing Swagger 2 schema, as backend codegen currently relies on that.

  • See if it works
    • Figure out Swagger UI
  • Check sum type representation
  • Manually test the functionality in commit "Add OpenAPI 3 non-trivial instances"

For #553

Retrievable with

nix run .#internal-hercules-gen-swagger -- --openapi3 | jq . >openapi3-draft.json

Not properly tested, but this is the right direction.
This + merge solves the problem of the openapi code going out of sync.
@roberth roberth marked this pull request as ready for review January 23, 2024 14:42
@roberth roberth enabled auto-merge January 23, 2024 14:42
@roberth roberth merged commit 56b76a6 into master Jan 23, 2024
3 checks passed
@roberth roberth deleted the openapi-3 branch January 23, 2024 14:47
@Kranzes
Copy link

Kranzes commented Jan 27, 2024

This does not pass the validation on https://editor.swagger.io/

@roberth
Copy link
Member Author

roberth commented Jan 29, 2024

Thanks, I didn't know about that.
Looks like the library put some parentheses in identifiers. It suggests that they can be percent encoded, but it's probably better to drop them altogether, because I don't think other tools will handle parentheses very well.

@Kranzes
Copy link

Kranzes commented Jan 29, 2024

Also theres like a ton of duplicated "jwt" things

@roberth
Copy link
Member Author

roberth commented May 3, 2024

I've made two fixes in #590.
It passes the validation on https://editor.swagger.io/ now.

@Kranzes
Copy link

Kranzes commented May 3, 2024

It seems like I still need operationId. Could you check if it can be added via the openapi3 library you're using for HCI?

Context for why I need it: oxidecomputer/progenitor#517 (comment)

@roberth
Copy link
Member Author

roberth commented May 4, 2024

Could you try #592?:

nix build hercules-ci-agent/openapi3-operationId#hercules-ci-api-openapi3

@Kranzes
Copy link

Kranzes commented May 4, 2024

The client creation no longer fails on that (fails on something else now). So I guess it's fine? One thing that looks wrong to me is the way you just use replace many times with hard-coded values. Maybe there's a nicer way of doing it that's more dynamic?

@roberth
Copy link
Member Author

roberth commented May 4, 2024

Upstream doesn't support it properly.

The string replacements are a bit ad hoc, but it's the most efficient way to add this information, until servant implements it properly.
Something more automated would probably lead to worse identfiers.

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.

2 participants