Conversation
|
/assign @ederign |
There was a problem hiding this comment.
Thanks for working on this, Pushpa! This is a good start. I wanted to share some guidance based on how we implemented the same feature in the notebooks backend, which can serve as a reference.
Swag Command Enhancements
The current command:
$(SWAG) init -g cmd/main.go -o ../api/openapi --outputTypes json,yaml --parseDependency --parseInternalSuggested improvements (see notebooks Makefile):
$(SWAG) fmt -g main.go -d $(SWAG_DIRS)
$(SWAG) init --parseDependency -q -g main.go -d $(SWAG_DIRS) --output openapi --outputTypes go,json,yaml --requiredByDefaultKey additions:
swag fmtbefore init - formats annotations consistently--requiredByDefault- marks fields as required by default (better API contracts)--outputTypes go,json,yaml- generatesdocs.gowhich is needed for serving Swagger UI and programmatic access-d $(SWAG_DIRS)- explicitly specify directories to scan
Build Integration
Currently make openapi is a standalone target. I assume this is because we are starting it, right?
build: fmt vet openapi ## swag runs on every build
run: fmt vet openapi ## swag runs on every run IN future, let's make sure to add this to the build. This ensures the spec is always up-to-date and prevents drift. Let's include this in next PRs? Maybe we can generate a file 'on the side' of the current one until we don't finishes it?
Definition Names
The generated spec has fully-qualified type names:
"github_com_kubeflow_model-registry_ui_bff_internal_models.HealthCheckModel"Compare to notebooks which generates clean names:
"health_check.HealthCheck"This may require reorganizing how types are imported/referenced. See how notebooks structures its models in internal/models/ subdirectories.
Annotation Completeness
When extending to all endpoints, consider including (see workspaces_handler.go for examples):
@Accept jsondirective- Path parameters with examples:
@Param namespace path string true "Namespace" extensions(x-example=kubeflow-user-example-com) - All relevant HTTP error codes (401, 403, 404, 422, 500)
- Detailed descriptions for each response
Optional: Swagger UI Handler
The notebooks backend includes a swagger_handler.go that serves interactive Swagger UI documentation. This is helpful for local development and API exploration.
Let me know if you have questions about any of these suggestions!
@ppadti and again, thank you so much! This is denivitelly on the right direction! Just small tweaks and we can start to expand it to all endpoints
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Signed-off-by: ppadti <ppadti@redhat.com>
Signed-off-by: ppadti <ppadti@redhat.com>
2ece8f6 to
27bf7f9
Compare
Signed-off-by: ppadti <ppadti@redhat.com>
Signed-off-by: ppadti <ppadti@redhat.com>
Description
This PR aims to generate the openapi spec. Added make cmd to generate open api spec and added checks in CI to test on each PR
Currently added for one handler to test and get reviews, once that is done, will convert all the endpoints
How Has This Been Tested?
make openapi cmd will generate swagger.yaml and swagger.json file for nowMerge criteria:
DCOcheck)ok-to-testhas been added to the PR.If you have UI changes