Skip to content

Commit

Permalink
chore: allow empty run metadata requests to delete existing metadata (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
corban-beaird committed Jun 18, 2024
1 parent 8006e2e commit 3320107
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 11 deletions.
4 changes: 0 additions & 4 deletions master/internal/api_runs.go
Original file line number Diff line number Diff line change
Expand Up @@ -943,10 +943,6 @@ func (a *apiServer) PostRunMetadata(
return nil, err
}

if len(req.Metadata.AsMap()) == 0 || req.Metadata.AsMap() == nil {
return nil, status.Error(codes.InvalidArgument, "metadata cannot be empty")
}

// Flatten Request Metadata.
flatMetadata, err := run.FlattenMetadata(req.Metadata.AsMap())
if err != nil {
Expand Down
16 changes: 16 additions & 0 deletions master/internal/api_runs_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1247,6 +1247,22 @@ func TestPostRunMetadata(t *testing.T) {
})
require.NoError(t, err)
require.Equal(t, rawMetadata, metadataResp.Metadata.AsMap())

// empty metadata
metadataResp, err = api.PostRunMetadata(ctx, &apiv1.PostRunMetadataRequest{
RunId: r.Id,
Metadata: &structpb.Struct{},
})
require.NoError(t, err)
require.Empty(t, metadataResp.Metadata.AsMap())

// nil metadata
metadataResp, err = api.PostRunMetadata(ctx, &apiv1.PostRunMetadataRequest{
RunId: r.Id,
Metadata: nil,
})
require.NoError(t, err)
require.Empty(t, metadataResp.Metadata.AsMap())
}

func TestRunMetadata(t *testing.T) {
Expand Down
5 changes: 5 additions & 0 deletions master/internal/db/postgres_runs.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ func UpdateRunMetadata(
if err != nil {
return fmt.Errorf("deleting run metadata indexes for run(%d): %w", runID, err)
}

// ignore if there is no metadata to insert.
if len(flatMetadata) == 0 {
return nil
}
_, err = tx.NewInsert().Model(&flatMetadata).Exec(ctx)
if err != nil {
return fmt.Errorf("inserting run metadata indexes for run(%d): %w", runID, err)
Expand Down
2 changes: 1 addition & 1 deletion master/internal/run/runs.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func FlattenMetadata(
data map[string]any,
) (flatMetadata []model.RunMetadataIndex, err error) {
if len(data) == 0 {
return nil, fmt.Errorf("metadata is empty")
return nil, nil
}
flatMetadata, err = flattenMetadata(data)
if err != nil {
Expand Down
12 changes: 6 additions & 6 deletions master/internal/run/runs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,15 @@ func TestFlattenMetadata(t *testing.T) {

func TestFlattenMetadataEmpty(t *testing.T) {
data := map[string]interface{}{}
_, err := FlattenMetadata(data)
require.Error(t, err, "metadata is empty")
flatMetadata, err := FlattenMetadata(data)
require.NoError(t, err)
require.Empty(t, flatMetadata)
}

// TODO(corban): this should actually be adjusted to return an error since we don't want to
// allow empty post requests to be made to the metadata endpoint.
func TestFlattenMetadataNil(t *testing.T) {
_, err := FlattenMetadata(nil)
require.Error(t, err, "metadata is empty")
flatMetadata, err := FlattenMetadata(nil)
require.NoError(t, err)
require.Empty(t, flatMetadata)
}

func TestFlattenMetadataNested(t *testing.T) {
Expand Down

0 comments on commit 3320107

Please sign in to comment.