Skip to content

Commit

Permalink
Revert "feat: Add regex search to task logs API (#9994)"
Browse files Browse the repository at this point in the history
This reverts commit f5400ea.
  • Loading branch information
gt2345 committed Nov 12, 2024
1 parent 2e076b7 commit 72e3124
Show file tree
Hide file tree
Showing 16 changed files with 812 additions and 943 deletions.
6 changes: 0 additions & 6 deletions harness/determined/common/api/bindings.py

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

2 changes: 0 additions & 2 deletions master/internal/api/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ const (
FilterOperationLessThanEqual
// FilterOperationStringContainment checks if the field contains a value as a substring.
FilterOperationStringContainment
// FilterOperationRegexContainment checks if the field contains the regex.
FilterOperationRegexContainment
)

// Filter is a general representation for a filter provided to an API.
Expand Down
18 changes: 5 additions & 13 deletions master/internal/api_tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -729,19 +729,11 @@ func constructTaskLogsFilters(req *apiv1.TaskLogsRequest) ([]api.Filter, error)
}

if req.SearchText != "" {
if req.EnableRegex {
filters = append(filters, api.Filter{
Field: "log",
Operation: api.FilterOperationRegexContainment,
Values: req.SearchText,
})
} else {
filters = append(filters, api.Filter{
Field: "log",
Operation: api.FilterOperationStringContainment,
Values: req.SearchText,
})
}
filters = append(filters, api.Filter{
Field: "log",
Operation: api.FilterOperationStringContainment,
Values: req.SearchText,
})
}
return filters, nil
}
Expand Down
17 changes: 0 additions & 17 deletions master/internal/api_tasks_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,23 +103,6 @@ func TestPostTaskLogs(t *testing.T) {
require.Equal(t, e.Source, a.Source)
require.Equal(t, e.Stdtype, a.Stdtype)
}

// Test log filtering by regex
stream = &mockStream[*apiv1.TaskLogsResponse]{ctx: ctx}
err = api.TaskLogs(&apiv1.TaskLogsRequest{
TaskId: string(task.TaskID),
SearchText: "^lo.{4}xt",
}, stream)
require.NoError(t, err)
require.Empty(t, stream.getData())

err = api.TaskLogs(&apiv1.TaskLogsRequest{
TaskId: string(task.TaskID),
SearchText: "^lo.{4}xt",
EnableRegex: true,
}, stream)
require.NoError(t, err)
require.Len(t, stream.getData(), 1)
}

func mockNotebookWithWorkspaceID(
Expand Down
1 change: 0 additions & 1 deletion master/internal/api_trials.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ func (a *apiServer) TrialLogs(
TimestampAfter: req.TimestampAfter,
OrderBy: req.OrderBy,
SearchText: req.SearchText,
EnableRegex: req.EnableRegex,
}, res)
err := processBatches(res, func(b api.Batch) error {
return b.ForEach(func(i interface{}) error {
Expand Down
2 changes: 0 additions & 2 deletions master/internal/db/postgres_filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ func filterToSQL(
return fmt.Sprintf("AND encode(%s::bytea, 'escape') ILIKE ('%%%%' || $%d || '%%%%')",
field,
paramID)
case api.FilterOperationRegexContainment:
return fmt.Sprintf("AND encode(%s::bytea, 'escape') ~ $%d", field, paramID)
default:
panic(fmt.Sprintf("cannot convert operation %d to SQL", f.Operation))
}
Expand Down
25 changes: 0 additions & 25 deletions master/internal/db/postgres_tasks_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -733,31 +733,6 @@ func TestTaskLogsFlow(t *testing.T) {
require.NoError(t, err)
require.Len(t, logs, 2)

// Filter by search text.
logs, _, err = db.TaskLogs(t1In.TaskID, 5, []api.Filter{{
Field: "log",
Operation: api.FilterOperationStringContainment,
Values: []string{"this"},
}}, apiv1.OrderBy_ORDER_BY_UNSPECIFIED, nil)
require.NoError(t, err)
require.Len(t, logs, 2)

logs, _, err = db.TaskLogs(t1In.TaskID, 5, []api.Filter{{
Field: "log",
Operation: api.FilterOperationStringContainment,
Values: []string{"^th.s"},
}}, apiv1.OrderBy_ORDER_BY_UNSPECIFIED, nil)
require.NoError(t, err)
require.Empty(t, logs)

logs, _, err = db.TaskLogs(t1In.TaskID, 5, []api.Filter{{
Field: "log",
Operation: api.FilterOperationRegexContainment,
Values: []string{"^th.s"},
}}, apiv1.OrderBy_ORDER_BY_UNSPECIFIED, nil)
require.NoError(t, err)
require.Len(t, logs, 2)

// Test DeleteTaskLogs.
err = db.DeleteTaskLogs([]model.TaskID{t2In.TaskID})
require.NoError(t, err)
Expand Down
10 changes: 1 addition & 9 deletions master/internal/elastic/elastic_task_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,15 +463,7 @@ func filtersToElastic(fs []api.Filter) []jsonObj {
},
},
})
case api.FilterOperationRegexContainment:
terms = append(terms,
jsonObj{
"regexp": jsonObj{
f.Field: jsonObj{
"value": fmt.Sprintf("%s", f.Values),
},
},
})

default:
panic(fmt.Sprintf("unsupported filter operation: %d", f.Operation))
}
Expand Down
Loading

0 comments on commit 72e3124

Please sign in to comment.