diff --git a/clients/ui/bff/README.md b/clients/ui/bff/README.md index 026453969..37028090a 100644 --- a/clients/ui/bff/README.md +++ b/clients/ui/bff/README.md @@ -110,17 +110,7 @@ curl -i localhost:4000/api/v1/model_registry/model-registry/registered_models/1 curl -i -X PATCH "http://localhost:4000/api/v1/model_registry/model-registry/registered_models/1" \ -H "Content-Type: application/json" \ -d '{ "data": { -"customProperties": { -"my-label9": { -"metadataType": "MetadataStringValue", -"string_value": "val" -} -}, -"description": "bella description", -"externalId": "9927", -"name": "bella", -"owner": "eder", -"state": "LIVE" + "description": "New description" }}' ``` ``` @@ -150,19 +140,8 @@ curl -i -X POST "http://localhost:4000/api/v1/model_registry/model-registry/mode # PATCH /api/v1/model_registry/{model_registry_id}/model_versions/{model_version_id} curl -i -X PATCH "http://localhost:4000/api/v1/model_registry/model-registry/model_versions/1" \ -H "Content-Type: application/json" \ - -d '{ "data": { - "customProperties": { - "my-label9": { - "metadataType": "MetadataStringValue", - "string_value": "val" - } - }, - "description": "New description", - "externalId": "9927", - "name": "ModelVersion One", - "state": "LIVE", - "author": "alex", - "registeredModelId": "1" +-d '{ "data": { + "description": "New description 2" }}' ``` ``` @@ -180,11 +159,12 @@ curl -i -X POST "http://localhost:4000/api/v1/model_registry/model-registry/regi "string_value": "val" } }, - "description": "New description", - "externalId": "9927", + "description": "Description", + "externalId": "9928", "name": "ModelVersion One", "state": "LIVE", "author": "alex" + "registeredModelId: "1" }}' ``` ``` diff --git a/clients/ui/bff/api/model_versions_handler.go b/clients/ui/bff/api/model_versions_handler.go index a68c25760..f8f779442 100644 --- a/clients/ui/bff/api/model_versions_handler.go +++ b/clients/ui/bff/api/model_versions_handler.go @@ -13,6 +13,8 @@ import ( type ModelVersionEnvelope Envelope[*openapi.ModelVersion, None] type ModelVersionListEnvelope Envelope[*openapi.ModelVersionList, None] +type ModelVersionUpdateEnvelope Envelope[*openapi.ModelVersionUpdate, None] + type ModelArtifactListEnvelope Envelope[*openapi.ModelArtifactList, None] type ModelArtifactEnvelope Envelope[*openapi.ModelArtifact, None] @@ -105,7 +107,7 @@ func (app *App) UpdateModelVersionHandler(w http.ResponseWriter, r *http.Request return } - var envelope ModelVersionEnvelope + var envelope ModelVersionUpdateEnvelope if err := json.NewDecoder(r.Body).Decode(&envelope); err != nil { app.serverErrorResponse(w, r, fmt.Errorf("error decoding JSON:: %v", err.Error())) return @@ -113,10 +115,7 @@ func (app *App) UpdateModelVersionHandler(w http.ResponseWriter, r *http.Request data := *envelope.Data - if err := validation.ValidateModelVersion(data); err != nil { - app.badRequestResponse(w, r, fmt.Errorf("validation error:: %v", err.Error())) - return - } + //TODO add validation - note updating requires different rules to create as fields are optional. jsonData, err := json.Marshal(data) if err != nil { diff --git a/clients/ui/bff/api/model_versions_handler_test.go b/clients/ui/bff/api/model_versions_handler_test.go index 27050e819..ffda872a4 100644 --- a/clients/ui/bff/api/model_versions_handler_test.go +++ b/clients/ui/bff/api/model_versions_handler_test.go @@ -37,7 +37,10 @@ func TestUpdateModelVersionHandler(t *testing.T) { data := mocks.GetModelVersionMocks()[0] expected := ModelVersionEnvelope{Data: &data} - body := ModelVersionEnvelope{Data: openapi.NewModelVersion("Model One", "1")} + reqData := openapi.ModelVersionUpdate{ + Description: openapi.PtrString("New description"), + } + body := ModelVersionUpdateEnvelope{Data: &reqData} actual, rs, err := setupApiTest[ModelVersionEnvelope](http.MethodPatch, "/api/v1/model_registry/model-registry/model_versions/1", body) assert.NoError(t, err) diff --git a/clients/ui/bff/api/registered_models_handler.go b/clients/ui/bff/api/registered_models_handler.go index 6a27215bc..21cf27f49 100644 --- a/clients/ui/bff/api/registered_models_handler.go +++ b/clients/ui/bff/api/registered_models_handler.go @@ -13,6 +13,7 @@ import ( type RegisteredModelEnvelope Envelope[*openapi.RegisteredModel, None] type RegisteredModelListEnvelope Envelope[*openapi.RegisteredModelList, None] +type RegisteredModelUpdateEnvelope Envelope[*openapi.RegisteredModelUpdate, None] func (app *App) GetAllRegisteredModelsHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { //TODO (ederign) implement pagination @@ -127,7 +128,7 @@ func (app *App) UpdateRegisteredModelHandler(w http.ResponseWriter, r *http.Requ return } - var envelope RegisteredModelEnvelope + var envelope RegisteredModelUpdateEnvelope if err := json.NewDecoder(r.Body).Decode(&envelope); err != nil { app.serverErrorResponse(w, r, fmt.Errorf("error decoding JSON:: %v", err.Error())) return @@ -135,10 +136,7 @@ func (app *App) UpdateRegisteredModelHandler(w http.ResponseWriter, r *http.Requ data := *envelope.Data - if err := validation.ValidateRegisteredModel(data); err != nil { - app.badRequestResponse(w, r, fmt.Errorf("validation error:: %v", err.Error())) - return - } + //TODO Validate body, note validation requirements are different to POST (fields are optional) jsonData, err := json.Marshal(data) if err != nil { @@ -146,7 +144,7 @@ func (app *App) UpdateRegisteredModelHandler(w http.ResponseWriter, r *http.Requ return } - patchedModel, err := app.modelRegistryClient.UpdateRegisteredModel(client, ps.ByName("id"), jsonData) + patchedModel, err := app.modelRegistryClient.UpdateRegisteredModel(client, ps.ByName(RegisteredModelId), jsonData) if err != nil { var httpErr *integrations.HTTPError if errors.As(err, &httpErr) { diff --git a/clients/ui/bff/api/registered_models_handler_test.go b/clients/ui/bff/api/registered_models_handler_test.go index 125be1cd3..133e22ab0 100644 --- a/clients/ui/bff/api/registered_models_handler_test.go +++ b/clients/ui/bff/api/registered_models_handler_test.go @@ -53,13 +53,17 @@ func TestUpdateRegisteredModelHandler(t *testing.T) { data := mocks.GetRegisteredModelMocks()[0] expected := RegisteredModelEnvelope{Data: &data} - body := RegisteredModelEnvelope{Data: openapi.NewRegisteredModel("Model One")} + reqData := openapi.RegisteredModelUpdate{ + Description: openapi.PtrString("This is a new description"), + } + body := RegisteredModelUpdateEnvelope{Data: &reqData} actual, rs, err := setupApiTest[RegisteredModelEnvelope](http.MethodPatch, "/api/v1/model_registry/model-registry/registered_models/1", body) assert.NoError(t, err) assert.Equal(t, http.StatusOK, rs.StatusCode) - assert.Equal(t, expected.Data.Name, actual.Data.Name) + //TODO when mock client can handle changing state, update this to verify the changes are made. + assert.Equal(t, expected.Data.Description, actual.Data.Description) } func TestGetAllModelVersionsForRegisteredModelHandler(t *testing.T) { diff --git a/clients/ui/bff/integrations/http.go b/clients/ui/bff/integrations/http.go index dbed3235a..6d58c63aa 100644 --- a/clients/ui/bff/integrations/http.go +++ b/clients/ui/bff/integrations/http.go @@ -111,5 +111,42 @@ func (c *HTTPClient) POST(url string, body io.Reader) ([]byte, error) { } func (c *HTTPClient) PATCH(url string, body io.Reader) ([]byte, error) { - return nil, fmt.Errorf("not implemented") + fullURL := c.baseURL + url + req, err := http.NewRequest(http.MethodPatch, fullURL, body) + if err != nil { + return nil, err + } + + req.Header.Set("Content-Type", "application/json") + req.Header.Add("Authorization", "Bearer "+c.bearerToken) + + response, err := c.client.Do(req) + if err != nil { + return nil, err + } + defer response.Body.Close() + + responseBody, err := io.ReadAll(response.Body) + if err != nil { + return nil, fmt.Errorf("error reading response body: %w", err) + } + + if response.StatusCode != http.StatusOK { + var errorResponse ErrorResponse + if err := json.Unmarshal(responseBody, &errorResponse); err != nil { + return nil, fmt.Errorf("error unmarshalling error response: %w", err) + } + httpError := &HTTPError{ + StatusCode: response.StatusCode, + ErrorResponse: errorResponse, + } + //Sometimes the code comes empty from model registry API + //also not all error codes are correctly implemented + //see https://github.com/kubeflow/model-registry/issues/95 + if httpError.ErrorResponse.Code == "" { + httpError.ErrorResponse.Code = strconv.Itoa(response.StatusCode) + } + return nil, httpError + } + return responseBody, nil }