fix(openapi): remove spec file generation from live code path #1290
Conversation
dnephin
force-pushed
the
dnephin/openapi-spec-gen
branch
from
729289c
to
193766a
Compare
| "example": "email@example.com", | ||
| "in": "query", | ||
| "name": "email", | ||
| "schema": { | ||
| "example": "email@example.com", |
There was a problem hiding this comment.
I was surprised to see these changes, but running the test on main showed the same changes.
I guess that the API definitions changed in another PR that was merged at the same time as the openapi spec generation changes.
| wd, err := os.Getwd() | ||
| require.NoError(t, err) | ||
|
|
||
| parts := strings.Split(wd, string(os.PathSeparator)) | ||
|
|
||
| if parts[len(parts)-1] != "infra" { | ||
| err := os.Chdir("../..") |
There was a problem hiding this comment.
go test always runs tests with the package directory as the working directory. So we can use a specific path instead of a conditional.
| @@ -92,8 +92,6 @@ func (a *API) registerRoutes(router *gin.RouterGroup) { | |||
|
|
|||
| get(unauthorized, "/version", a.Version) | |||
| } | |||
|
|
|||
| generateOpenAPI() | |||
There was a problem hiding this comment.
This was the main goal of this PR, to remove this from the production code path.
| s.GenerateRoutes() | ||
|
|
||
| filename := "../../docs/api/openapi3.json" |
There was a problem hiding this comment.
you can't assume the test will always run from this directory.
There was a problem hiding this comment.
See my comment, you can! (although I can't find a good docs link right now, I'll keep looking)
There was a problem hiding this comment.
I guess I'm mistaken. I tested different run methods and couldn't find a problem.
There was a problem hiding this comment.
I'm not sure where it is documented, but I've always seen it work this way. Somewhere in go test it checked to the directory for the package before running the test binary.
| @@ -3321,11 +3321,9 @@ | |||
| "operationId": "ListUsers", | |||
| "parameters": [ | |||
| { | |||
| "example": "email@example.com", | |||
There was a problem hiding this comment.
this is weird. should not have changed.
There was a problem hiding this comment.
See my comment below :)
It happens on main as well.
There was a problem hiding this comment.
I checked this and you are correct, it was removed in main and not regenerated
| @@ -3321,11 +3321,9 @@ | |||
| "operationId": "ListUsers", | |||
| "parameters": [ | |||
| { | |||
| "example": "email@example.com", | |||
There was a problem hiding this comment.
See my comment below :)
It happens on main as well.
| s.GenerateRoutes() | ||
|
|
||
| filename := "../../docs/api/openapi3.json" |
There was a problem hiding this comment.
See my comment, you can! (although I can't find a good docs link right now, I'll keep looking)
dnephin
force-pushed
the
dnephin/openapi-spec-gen
branch
2 times, most recently
from
846e1a0
to
e6ade05
Compare
Previously it could have run if the binary was run from any directory which contains a `docs` or `.git` directory.
Generating the openapi spec is a developer operation, not an end-user operation. This PR removes the sub-command from the CLI and adds a new internal/openapigen command. This matches the pattern we use for generating the user documentation (internal/docsgen).
dnephin
force-pushed
the
dnephin/openapi-spec-gen
branch
from
e6ade05
to
ba10e92
Compare
Summary
This PR removes the openapi spec generation from the production CLI and removes the possibility that it could run when the server starts. We continue to run the spec generation in tests, and it can be run manually from the makefile.
More comments inline.
Related Issues
Builds on #1270, and #1207