Skip to content

Commit

Permalink
Adds support for PATCH requests to work outside of mock mode
Browse files Browse the repository at this point in the history
Signed-off-by: Alex Creasy <[email protected]>
  • Loading branch information
alexcreasy committed Sep 18, 2024
1 parent 20561b1 commit ff875a3
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 41 deletions.
32 changes: 6 additions & 26 deletions clients/ui/bff/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}}'
```
```
Expand Down Expand Up @@ -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"
}}'
```
```
Expand All @@ -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"
}}'
```
```
Expand Down
9 changes: 4 additions & 5 deletions clients/ui/bff/api/model_versions_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down Expand Up @@ -105,18 +107,15 @@ 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
}

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 {
Expand Down
5 changes: 4 additions & 1 deletion clients/ui/bff/api/model_versions_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 4 additions & 6 deletions clients/ui/bff/api/registered_models_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -127,26 +128,23 @@ 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
}

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 {
app.serverErrorResponse(w, r, fmt.Errorf("error marshaling model to JSON: %w", err))
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) {
Expand Down
8 changes: 6 additions & 2 deletions clients/ui/bff/api/registered_models_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
39 changes: 38 additions & 1 deletion clients/ui/bff/integrations/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit ff875a3

Please sign in to comment.