Skip to content

Commit

Permalink
docs: API deprecate returning config for bulk endpoints (#8732)
Browse files Browse the repository at this point in the history
  • Loading branch information
NicholasBlaskey authored Jan 25, 2024
1 parent f21a516 commit d68ffaa
Show file tree
Hide file tree
Showing 10 changed files with 1,011 additions and 931 deletions.
9 changes: 9 additions & 0 deletions docs/release-notes/bulk-endpoint-experiment-config.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
:orphan:

**Deprecated Features**

- API: The experiment API object in a future version will have its ``config`` field removed to
improve performance of the system. A new ``config`` field is added now to the response of
``/api/v1/experiments/{experiment_id}`` that can be used as a replacement.

If you are not calling the APIs manually there will be no impact to you.
8 changes: 8 additions & 0 deletions harness/determined/common/api/bindings.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions master/internal/api_experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,10 @@ func (a *apiServer) GetExperiment(
return nil, err
}

// Update this when we remove the proto type.
resp := apiv1.GetExperimentResponse{
Experiment: exp,
Config: exp.Config, //nolint:staticcheck
}

// Only continue to add a job summary if it's an active experiment.
Expand Down Expand Up @@ -2686,6 +2688,8 @@ func (a *apiServer) SearchExperiments(
// Correct trial restarts because
// `restart` count is incremented before `restart <= max_restarts` stop restart check,
// so trials in terminal state have restarts = max + 1.
// Update this correction to happen in the database when we do the remove.
//nolint:staticcheck
configRestarts, ok := experiment.Config.Fields["max_restarts"].AsInterface().(float64)
if ok && trial.Restarts > int32(configRestarts) {
trial.Restarts = int32(configRestarts)
Expand Down
30 changes: 29 additions & 1 deletion master/internal/api_experiment_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,34 @@ func TestGetExperimentLabels(t *testing.T) {
require.Subset(t, resp.Labels, labels)
}

func TestGetExperimentConfig(t *testing.T) {
api, curUser, ctx := setupAPITest(t, nil)

exp := createTestExp(t, api, curUser)
expectedBytes, err := db.SingleDB().ExperimentConfigRaw(exp.ID)
require.NoError(t, err)
expected := make(map[string]any)
require.NoError(t, json.Unmarshal(expectedBytes, &expected))

resp, err := api.GetExperiment(ctx, &apiv1.GetExperimentRequest{
ExperimentId: int32(exp.ID),
})
require.NoError(t, err)

cases := []struct {
name string
config *structpb.Struct
}{
{"GetExperimentResponse.Config", resp.Config},
{"GetExperimentResponse.Experiment.Config", resp.Experiment.Config}, //nolint:staticcheck
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
require.Equal(t, expected, c.config.AsMap())
})
}
}

func TestGetTaskContextDirectoryExperiment(t *testing.T) {
api, curUser, ctx := setupAPITest(t, nil)

Expand Down Expand Up @@ -744,7 +772,7 @@ func getExperimentsTest(ctx context.Context, t *testing.T, api *apiServer, pid i
sort.Strings(res.Experiments[i].Labels)

// Don't compare config.
res.Experiments[i].Config = nil
res.Experiments[i].Config = nil //nolint:staticcheck

// Compare time seperatly due to millisecond precision in postgres.
require.WithinDuration(t,
Expand Down
2 changes: 2 additions & 0 deletions master/pkg/model/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,8 @@ func ExperimentFromProto(e *experimentv1.Experiment) (*Experiment, error) {
parentID = ptrs.Ptr(int(e.ForkedFrom.Value))
}

// Update this when we remove the proto type.
//nolint:staticcheck
byts, err := json.Marshal(e.Config)
if err != nil {
return nil, err
Expand Down
Loading

0 comments on commit d68ffaa

Please sign in to comment.