Move SearchExperiments endpoint #68
Conversation
Signed-off-by: Software Developer <7852635+dsuhinin@users.noreply.github.com>
Signed-off-by: Software Developer <7852635+dsuhinin@users.noreply.github.com>
Signed-off-by: Software Developer <7852635+dsuhinin@users.noreply.github.com>
Signed-off-by: Software Developer <7852635+dsuhinin@users.noreply.github.com>
| @@ -9,6 +9,7 @@ linters: | |||
| - gosimple | |||
| - lll | |||
| disable: | |||
| - ireturn | |||
There was a problem hiding this comment.
disabled it. sometimes it bothers.
There was a problem hiding this comment.
@fabiovincenzi can you update Go for us to deal with this problem?
There was a fix for this, but only starting Go 1.23 I believe.
This is indeed annoying.
| @@ -34,6 +34,7 @@ var validations = map[string]string{ | |||
| "SetExperimentTag_ExperimentId": "required", | |||
| "SetExperimentTag_Key": "required,max=250,validMetricParamOrTagName", | |||
| "SetExperimentTag_Value": "max=5000", | |||
| "SearchExperiments_MaxResults": "positiveNonZeroInteger,max=50000", | |||
There was a problem hiding this comment.
positiveNonZeroInteger because of Python tests.
| def test_search_experiments_max_results_validation(store: SqlAlchemyStore): | ||
| with pytest.raises( | ||
| MlflowException, | ||
| match=r"Invalid value 0 for parameter 'max_results' supplied", | ||
| ): | ||
| store.search_experiments(max_results=0) | ||
| with pytest.raises( | ||
| MlflowException, | ||
| match=r"Invalid value 1000000 for parameter 'max_results' supplied", | ||
| ): | ||
| store.search_experiments(max_results=1_000_000) |
There was a problem hiding this comment.
One sub tests removed -> related to None value in max_results. Other tests looks the same but expected messages a bit changed.
Signed-off-by: Software Developer <7852635+dsuhinin@users.noreply.github.com>
Signed-off-by: Software Developer <7852635+dsuhinin@users.noreply.github.com>
| return query, nil | ||
| } | ||
|
|
||
| func createExperimentsNextPageToken(experiments []models.Experiment, limit, offset int) (string, *contract.Error) { |
There was a problem hiding this comment.
I'm not sure and I'm just asking, but is this the same next token thing as in search_runs? Can we reuse or did it not seem worth it?
There was a problem hiding this comment.
unfortunately not the same. it has a bit strange condition under the hood for experiments. Ive tried to make it generic but it seems like the way which works - to keep the separate.
Signed-off-by: Software Developer <7852635+dsuhinin@users.noreply.github.com>
| entity, key, comparison := parts[1], strings.Trim(parts[2], "\"`"), parts[3] | ||
|
|
||
| switch entity { | ||
| case "", "attribute", "attributes", "attr": |
There was a problem hiding this comment.
I have some of these strings as constants in /pkg/tracking/service/query/parser/validate.go.
Perhaps we want a more generate mlflow constants package at some point?
Could be addressed in the future though.
There was a problem hiding this comment.
Ill check and re use them if it possible. np.
There was a problem hiding this comment.
ahhh, they are exists in other packages on even different level. then I suggest to do not do that. packages are different, logic also and they are not in the same scope. Maybe later we can try to unify something and move common parts into the same layer.
Signed-off-by: Software Developer <7852635+dsuhinin@users.noreply.github.com>
Signed-off-by: Software Developer <7852635+dsuhinin@users.noreply.github.com>
No description provided.