Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filtering ability for all backend API ListXXX requests #537

Merged
merged 12 commits into from
Dec 14, 2018
29 changes: 17 additions & 12 deletions Gopkg.lock

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

4 changes: 4 additions & 0 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,7 @@ required = [
name = "google.golang.org/genproto"
revision = "bd91e49a0898e27abb88c339b432fa53d7497ac0"


[[constraint]]
name = "github.com/google/go-cmp"
version = "0.2.0"
3 changes: 3 additions & 0 deletions backend/api/experiment.proto
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ syntax = "proto3";

package api;

import "filter.proto";
import "google/api/annotations.proto";
import "google/protobuf/empty.proto";
import "google/protobuf/timestamp.proto";
Expand Down Expand Up @@ -96,6 +97,8 @@ message ListExperimentsRequest {
// Can be format of "field_name", "field_name asc" or "field_name des"
// Ascending by default.
string sort_by = 3;

Filter filter = 4;
}

message ListExperimentsResponse {
Expand Down
117 changes: 117 additions & 0 deletions backend/api/filter.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Copyright 2018 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

syntax = "proto3";

package api;

import "google/protobuf/timestamp.proto";

// Predicate captures individual conditions that must be true for a resource
// being filtered.
message Predicate {
// Op is the operation to apply.
enum Op {
UNKNOWN = 0;

// Operators on scalar values. Only applies to one of |int_value|,
// |long_value|, |string_value| or |timestamp_value|.
EQUALS = 1;
NOT_EQUALS = 2;
GREATER_THAN = 3;
GREATER_THAN_EQUALS = 5;
LESS_THAN = 6;
LESS_THAN_EQUALS = 7;

// Checks if the value is a member of a given array, which should be one of
// |int_values|, |long_values| or |string_values|.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But not timestamp_value? If there is a good reason for this, we should point it out here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no good reason other than I thought it unlikely we'd want to query specific dates vs a range of dates. If we ever want to do so, we can add it easily.

IN = 8;
}
Op op = 1;

string key = 2;
oneof value {
int32 int_value = 3;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed for a service interface. Clients usually aren't very careful about numeric type accuracy (not to mention our clients are dynamically typed languages anyway).
Is it safe to say that unless this proto is to be used between backend services, it doesn't make sense to use integers anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. The client can be in any language that supports protos, like Go. In which case types do matter.

int64 long_value = 4;
string string_value = 5;

// Timestamp values will be converted to Unix time (seconds since the epoch)
// prior to being used in a filtering operation.
google.protobuf.Timestamp timestamp_value = 6;

// Array values below are only meant to be used by the IN operator.
IntValues int_values = 7;
LongValues long_values = 8;
StringValues string_values = 9;
}
}

message IntValues {
repeated int32 values = 1;
}

message StringValues {
repeated string values = 2;
}

message LongValues {
repeated int64 values = 3;
}

// Filter is used to filter resources returned from a ListXXX request.
//
// Example filters:
// 1) Filter runs with status = 'Running'
// filter {
// predicate {
// key: "status"
// op: EQUALS
// string_value: "Running"
// }
// }
//
// 2) Filter runs that succeeded since Dec 1, 2018
// filter {
// predicate {
// key: "status"
// op: EQUALS
// string_value: "Succeeded"
// }
// predicate {
// key: "created_at"
// op: GREATER_THAN
// timestamp_value {
// seconds: 1543651200
// }
// }
// }
//
// 3) Filter runs with one of labels 'label_1' or 'label_2'
//
// filter {
// predicate {
// key: "label"
// op: IN
// string_values {
// value: 'label_1'
// value: 'label_2'
// }
// }
// }
message Filter {
// All predicates are AND-ed when this filter is applied.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Does this description really belong here? This seems to me like a backend logic contract rather than protocol, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic of the filter is part of the contract, so I think having it here is useful for anyone trying to create a Filter proto.

repeated Predicate predicates = 1;
}


84 changes: 47 additions & 37 deletions backend/api/go_client/experiment.pb.go

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

Loading