From fe65b02a5d18da3e244da65b78bd89dfe0673df6 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Wed, 20 May 2020 12:48:56 +0800 Subject: [PATCH 01/13] Added TestGetOnlineFeatures() client_test to test client.GetOnlineFeatures() --- sdk/go/client_test.go | 95 +++++++++++++++++++++++++++++++++++++++++++ sdk/go/go.mod | 1 + sdk/go/go.sum | 8 ++++ 3 files changed, 104 insertions(+) create mode 100644 sdk/go/client_test.go diff --git a/sdk/go/client_test.go b/sdk/go/client_test.go new file mode 100644 index 0000000000..a1902c500c --- /dev/null +++ b/sdk/go/client_test.go @@ -0,0 +1,95 @@ +package feast + +import ( + "context" + "testing" + + "github.com/feast-dev/feast/sdk/go/mocks" + "github.com/feast-dev/feast/sdk/go/protos/feast/serving" + "github.com/feast-dev/feast/sdk/go/protos/feast/types" + "github.com/golang/mock/gomock" + "github.com/google/go-cmp/cmp" + "github.com/opentracing/opentracing-go" +) + +func TestGetOnlineFeatures(t *testing.T) { + tt := []struct { + name string + req OnlineFeaturesRequest + recieve OnlineFeaturesResponse + want OnlineFeaturesResponse + wantErr bool + err error + }{ + { + name: "valid", + req: OnlineFeaturesRequest{ + Features: []string{ + "driver:rating", + "rating", + }, + Entities: []Row{ + {"driver_id": Int64Val(1)}, + }, + Project: "driver_project", + }, + // check GetOnlineFeatures() should strip projects returned from serving + recieve: OnlineFeaturesResponse{ + RawResponse: &serving.GetOnlineFeaturesResponse{ + FieldValues: []*serving.GetOnlineFeaturesResponse_FieldValues{ + { + Fields: map[string]*types.Value{ + "driver_project/driver:rating": Int64Val(1), + "driver_project/rating": Int64Val(1), + }, + }, + }, + }, + }, + want: OnlineFeaturesResponse{ + RawResponse: &serving.GetOnlineFeaturesResponse{ + FieldValues: []*serving.GetOnlineFeaturesResponse_FieldValues{ + { + Fields: map[string]*types.Value{ + "driver:rating": Int64Val(1), + "rating": Int64Val(1), + }, + }, + }, + }, + }, + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + // mock feast grpc client get online feature requestss + ctrl := gomock.NewController(t) + defer ctrl.Finish() + cli := mock_serving.NewMockServingServiceClient(ctrl) + ctx := context.Background() + _, traceCtx := opentracing.StartSpanFromContext(ctx, "get_online_features") + rawRequest, _ := tc.req.buildRequest() + resp := tc.recieve.RawResponse + cli.EXPECT().GetOnlineFeatures(traceCtx, rawRequest).Return(resp, nil).Times(1) + + client := &GrpcClient{ + cli: cli, + } + got, err := client.GetOnlineFeatures(ctx, &tc.req) + + if err != nil && !tc.wantErr { + t.Errorf("error = %v, wantErr %v", err, tc.wantErr) + return + } + if tc.wantErr && err.Error() != tc.err.Error() { + t.Errorf("error = %v, expected err = %v", err, tc.err) + return + } + // TODO: compare directly once OnlineFeaturesResponse no longer embeds a rawResponse. + if !cmp.Equal(got.RawResponse.String(), tc.want.RawResponse.String()) { + t.Errorf("got: \n%v\nwant:\n%v", got.RawResponse.String(), tc.want.RawResponse.String()) + } + }) + } +} diff --git a/sdk/go/go.mod b/sdk/go/go.mod index 06bd693616..656810354b 100644 --- a/sdk/go/go.mod +++ b/sdk/go/go.mod @@ -3,6 +3,7 @@ module github.com/feast-dev/feast/sdk/go go 1.13 require ( + github.com/golang/mock v1.4.3 github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0 github.com/google/go-cmp v0.4.0 github.com/opentracing/opentracing-go v1.1.0 diff --git a/sdk/go/go.sum b/sdk/go/go.sum index f08f302617..39e6ef923b 100644 --- a/sdk/go/go.sum +++ b/sdk/go/go.sum @@ -12,7 +12,10 @@ github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekf github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6 h1:ZgQEtGgCBiWRM39fZuwSd1LwSqqSW0hOdXCYYDX0R3I= github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= +github.com/golang/mock v1.1.1 h1:G5FRp8JnTd7RQH5kemVNlMeyXQAztQ3mOWV95KxsXH8= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= +github.com/golang/mock v1.4.3 h1:GV+pQPG/EUUbkh47niozDcADz6go/dUwhVzdUQHIVRw= +github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs= @@ -63,6 +66,7 @@ golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190502145724-3ef323f4f1fd h1:r7DufRZuZbWB7j439YfAzP8RPDa9unLkpwQKUYbIMPI= golang.org/x/sys v0.0.0-20190502145724-3ef323f4f1fd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs= @@ -71,6 +75,8 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY= golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= +golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= +golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 h1:5Beo0mZN8dRzgrMMkDp0jc8YXQKx9DiJ2k1dkvGsn5A= golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= @@ -100,3 +106,5 @@ gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= +rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= +rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= From 20bcddde3f65aef1a67d7cfa767b63560f738135 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Wed, 20 May 2020 17:49:35 +0800 Subject: [PATCH 02/13] Added unit test for Java SDK's FeastClient --- sdk/java/pom.xml | 26 +++- .../java/com/gojek/feast/FeastClientTest.java | 139 ++++++++++++++++++ 2 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java diff --git a/sdk/java/pom.xml b/sdk/java/pom.xml index 75d82edcc0..7856afd47e 100644 --- a/sdk/java/pom.xml +++ b/sdk/java/pom.xml @@ -18,6 +18,7 @@ 5.5.2 + 2.28.2 @@ -40,6 +41,10 @@ io.grpc grpc-stub + + io.grpc + grpc-testing + com.google.protobuf protobuf-java-util @@ -55,7 +60,7 @@ slf4j-api - + org.junit.jupiter junit-jupiter-engine @@ -80,7 +85,24 @@ 3.6 compile - + + org.mockito + mockito-core + ${mockito.version} + test + + + org.mockito + mockito-inline + ${mockito.version} + test + + + org.junit.vintage + junit-vintage-engine + ${junit.version} + test + diff --git a/sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java b/sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java new file mode 100644 index 0000000000..eac4deff43 --- /dev/null +++ b/sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java @@ -0,0 +1,139 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2019 The Feast Authors + * + * 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 + * + * https://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. + */ +package com.gojek.feast; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.AdditionalAnswers.delegatesTo; +import static org.mockito.Mockito.mock; + +import com.google.protobuf.Timestamp; +import feast.proto.serving.ServingAPIProto.FeatureReference; +import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesRequest; +import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesRequest.EntityRow; +import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesResponse; +import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesResponse.FieldValues; +import feast.proto.serving.ServingServiceGrpc.ServingServiceImplBase; +import feast.proto.types.ValueProto.Value; +import io.grpc.ManagedChannel; +import io.grpc.Status; +import io.grpc.inprocess.InProcessChannelBuilder; +import io.grpc.inprocess.InProcessServerBuilder; +import io.grpc.stub.StreamObserver; +import io.grpc.testing.GrpcCleanupRule; + +import java.time.Instant; +import java.util.List; +import java.util.Map; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +public class FeastClientTest { + @Rule public GrpcCleanupRule grpcRule; + private ServingServiceImplBase servingMock = + mock( + ServingServiceImplBase.class, + delegatesTo( + new ServingServiceImplBase() { + @Override + public void getOnlineFeatures( + GetOnlineFeaturesRequest request, + StreamObserver responseObserver) { + if (!request.equals(FeastClientTest.getFakeRequest())) { + responseObserver.onError(Status.UNKNOWN.asRuntimeException()); + } + + responseObserver.onNext(FeastClientTest.getFakeResponse()); + responseObserver.onCompleted(); + } + })); + private FeastClient client; + + @Before + public void setup() throws Exception { + this.grpcRule = new GrpcCleanupRule(); + // setup fake serving service + String serverName = InProcessServerBuilder.generateName(); + this.grpcRule.register( + InProcessServerBuilder.forName(serverName) + .directExecutor() + .addService(this.servingMock) + .build() + .start()); + + // setup test feast client target + ManagedChannel channel = + this.grpcRule.register( + InProcessChannelBuilder.forName(serverName).directExecutor().build()); + this.client = new FeastClient(channel); + } + + @Test + public void shouldGetOnlineFeatures() { + List rows = + this.client.getOnlineFeatures( + List.of("driver:name", "rating"), + List.of(Row.create().set("driver_id", 1).setEntityTimestamp(Instant.ofEpochSecond(100))), + "driver_project"); + + assertEquals( + rows.get(0).getFields(), + Map.of( + "driver_id", intValue(1), + "driver:name", strValue("david"), + "rating", intValue(3))); + } + + private static GetOnlineFeaturesRequest getFakeRequest() { + // setup mock serving service stub + return GetOnlineFeaturesRequest.newBuilder() + .addFeatures( + FeatureReference.newBuilder() + .setProject("driver_project") + .setFeatureSet("driver") + .setName("name") + .build()) + .addFeatures( + FeatureReference.newBuilder().setProject("driver_project").setName("rating").build()) + .addEntityRows( + EntityRow.newBuilder() + .setEntityTimestamp(Timestamp.newBuilder().setSeconds(100)) + .putFields("driver_id", intValue(1))) + .build(); + } + + private static GetOnlineFeaturesResponse getFakeResponse() { + return GetOnlineFeaturesResponse.newBuilder() + .addFieldValues( + FieldValues.newBuilder() + .putAllFields( + Map.of( + "driver_id", intValue(1), + "driver:name", strValue("david"), + "rating", intValue(3))) + .build()) + .build(); + } + + private static Value strValue(String val) { + return Value.newBuilder().setStringVal(val).build(); + } + + private static Value intValue(int val) { + return Value.newBuilder().setInt32Val(val).build(); + } +} From 21f41c9513ccb297b3a3cba858f88a0216bd2d6e Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Wed, 20 May 2020 18:31:50 +0800 Subject: [PATCH 03/13] Make Python SDK's client.get_online_features() unit test more comprehensive --- .../java/com/gojek/feast/FeastClientTest.java | 12 ++-- sdk/python/feast/client.py | 21 +++++-- sdk/python/tests/test_client.py | 62 ++++++++++--------- 3 files changed, 54 insertions(+), 41 deletions(-) diff --git a/sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java b/sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java index eac4deff43..f75a60832d 100644 --- a/sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java +++ b/sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java @@ -34,7 +34,6 @@ import io.grpc.inprocess.InProcessServerBuilder; import io.grpc.stub.StreamObserver; import io.grpc.testing.GrpcCleanupRule; - import java.time.Instant; import java.util.List; import java.util.Map; @@ -87,7 +86,8 @@ public void shouldGetOnlineFeatures() { List rows = this.client.getOnlineFeatures( List.of("driver:name", "rating"), - List.of(Row.create().set("driver_id", 1).setEntityTimestamp(Instant.ofEpochSecond(100))), + List.of( + Row.create().set("driver_id", 1).setEntityTimestamp(Instant.ofEpochSecond(100))), "driver_project"); assertEquals( @@ -121,10 +121,10 @@ private static GetOnlineFeaturesResponse getFakeResponse() { .addFieldValues( FieldValues.newBuilder() .putAllFields( - Map.of( - "driver_id", intValue(1), - "driver:name", strValue("david"), - "rating", intValue(3))) + Map.of( + "driver_id", intValue(1), + "driver:name", strValue("david"), + "rating", intValue(3))) .build()) .build(); } diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index 1c34213ad8..e62ef314d8 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -639,6 +639,18 @@ def get_online_features( self._connect_serving() try: + print("====================== actual:") + print( + str( + GetOnlineFeaturesRequest( + features=_build_feature_references( + feature_ref_strs=feature_refs, + project=project if project is not None else self.project, + ), + entity_rows=entity_rows, + ) + ) + ) response = self._serving_service_stub.GetOnlineFeatures( GetOnlineFeaturesRequest( features=_build_feature_references( @@ -658,14 +670,11 @@ def get_online_features( # strip the project part the string feature references returned from serving strip_fields = {} for ref_str, value in field_value.fields.items(): - # find and ignore entities - if ref_str in entity_refs: - strip_fields[ref_str] = value - else: - strip_ref_str = repr( + if ref_str not in entity_refs: + ref_str = repr( FeatureRef.from_str(ref_str, ignore_project=True) ) - strip_fields[strip_ref_str] = value + strip_fields[ref_str] = value strip_field_values.append( GetOnlineFeaturesResponse.FieldValues(fields=strip_fields) ) diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 380557ce92..b7331df56f 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -47,11 +47,12 @@ from feast.core.Source_pb2 import KafkaSourceConfig, Source, SourceType from feast.core.Store_pb2 import Store from feast.entity import Entity -from feast.feature_set import Feature, FeatureSet, FeatureSetRef +from feast.feature import Feature +from feast.feature_set import FeatureSet, FeatureSetRef from feast.job import IngestJob +from feast.serving.ServingService_pb2 import DataFormat, FeastServingType +from feast.serving.ServingService_pb2 import FeatureReference as FeatureRefProto from feast.serving.ServingService_pb2 import ( - DataFormat, - FeastServingType, GetBatchFeaturesResponse, GetFeastServingInfoResponse, GetJobResponse, @@ -201,7 +202,7 @@ def test_version(self, mocked_client, mocker): [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], ) def test_get_online_features(self, mocked_client, mocker): - ROW_COUNT = 300 + ROW_COUNT = 1 mocked_client._serving_service_stub = Serving.ServingServiceStub( grpc.insecure_channel("") @@ -210,44 +211,47 @@ def test_get_online_features(self, mocked_client, mocker): def int_val(x): return ValueProto.Value(int64_val=x) - # serving can return feature references with projects, - # get_online_features() should strip the project part. - field_values = GetOnlineFeaturesResponse.FieldValues( - fields={ - "driver_project/driver:driver_id": int_val(1), - "driver_project/driver_id": int_val(9), - } + request = GetOnlineFeaturesRequest() + request.features.extend( + [ + FeatureRefProto( + project="driver_project", feature_set="driver", name="age" + ), + FeatureRefProto(project="driver_project", name="rating"), + ] ) - - response = GetOnlineFeaturesResponse() - entity_rows = [] + expected_response = GetOnlineFeaturesResponse() for row_number in range(1, ROW_COUNT + 1): - response.field_values.append(field_values) - entity_rows.append( + request.entity_rows.append( GetOnlineFeaturesRequest.EntityRow( - fields={"customer_id": int_val(row_number)} + fields={"driver_id": int_val(row_number)} ) + ), + field_values = GetOnlineFeaturesResponse.FieldValues( + fields={ + "driver_id": int_val(row_number), + "driver_project/driver:age": int_val(1), + "driver_project/rating": int_val(9), + } ) + expected_response.field_values.append(field_values) mocker.patch.object( mocked_client._serving_service_stub, "GetOnlineFeatures", - return_value=response, + return_value=expected_response, ) - - # NOTE: Feast Serving does not allow for feature references - # that specify the same feature in the same request - response = mocked_client.get_online_features( - entity_rows=entity_rows, - feature_refs=["driver:driver_id", "driver_id"], + got_response = mocked_client.get_online_features( + entity_rows=request.entity_rows, + feature_refs=["driver:age", "rating"], project="driver_project", ) # type: GetOnlineFeaturesResponse - - assert ( - response.field_values[0].fields["driver:driver_id"].int64_val == 1 - and response.field_values[0].fields["driver_id"].int64_val == 9 + mocked_client._serving_service_stub.GetOnlineFeatures.assert_called_with( + request ) + assert got_response == expected_response + @pytest.mark.parametrize( "mocked_client", [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], @@ -635,7 +639,7 @@ def test_feature_set_ingest_throws_exception_if_kafka_down( ) with pytest.raises(exception): - test_client.ingest("driver-feature-set", dataframe) + test_client.ingest("driver-feature-set", dataframe, timeout=1) @pytest.mark.parametrize( "dataframe,exception,test_client", From 1e3604421d197ef111758dc177a0353a7a335d05 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Thu, 21 May 2020 09:14:29 +0800 Subject: [PATCH 04/13] Remove code added to debug the Python SDK tests --- sdk/python/feast/client.py | 12 ------------ sdk/python/tests/test_client.py | 12 ++++++++---- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index e62ef314d8..f10204c59d 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -639,18 +639,6 @@ def get_online_features( self._connect_serving() try: - print("====================== actual:") - print( - str( - GetOnlineFeaturesRequest( - features=_build_feature_references( - feature_ref_strs=feature_refs, - project=project if project is not None else self.project, - ), - entity_rows=entity_rows, - ) - ) - ) response = self._serving_service_stub.GetOnlineFeatures( GetOnlineFeaturesRequest( features=_build_feature_references( diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index b7331df56f..9fe11ae9d3 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -15,6 +15,7 @@ import pkgutil import tempfile +from copy import deepcopy from concurrent import futures from datetime import datetime from unittest import mock @@ -220,7 +221,7 @@ def int_val(x): FeatureRefProto(project="driver_project", name="rating"), ] ) - expected_response = GetOnlineFeaturesResponse() + recieve_response = GetOnlineFeaturesResponse() for row_number in range(1, ROW_COUNT + 1): request.entity_rows.append( GetOnlineFeaturesRequest.EntityRow( @@ -234,12 +235,12 @@ def int_val(x): "driver_project/rating": int_val(9), } ) - expected_response.field_values.append(field_values) + recieve_response.field_values.append(field_values) mocker.patch.object( mocked_client._serving_service_stub, "GetOnlineFeatures", - return_value=expected_response, + return_value=recieve_response, ) got_response = mocked_client.get_online_features( entity_rows=request.entity_rows, @@ -250,7 +251,10 @@ def int_val(x): request ) - assert got_response == expected_response + got_fields = got_response.field_values[0].fields + assert (got_fields["driver_id"] == int_val(1) + and got_fields["driver:age"] == int_val(1) + and got_fields["rating"] == int_val(9)) @pytest.mark.parametrize( "mocked_client", From e414a2d4671f6d2e3b9148aa42aac124e4cfd8d9 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Thu, 21 May 2020 09:42:08 +0800 Subject: [PATCH 05/13] Change repo referenced in go.mod to mrzzy fork to test modules retrieval --- go.mod | 4 ++-- sdk/go/go.mod | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 1be2396f6e..557d4ddec5 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,10 @@ -module github.com/feast-dev/feast +module github.com/mrzzy/feast require ( github.com/Masterminds/goutils v1.1.0 // indirect github.com/Masterminds/semver v1.5.0 // indirect github.com/Masterminds/sprig v2.22.0+incompatible // indirect - github.com/feast-dev/feast/sdk/go v0.0.0-20200516052424-09ff3dda724c // indirect + github.com/mrzzy/feast/sdk/go v0.0.0-20200516052424-09ff3dda724c // indirect github.com/ghodss/yaml v1.0.0 github.com/gogo/protobuf v1.3.1 // indirect github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect diff --git a/sdk/go/go.mod b/sdk/go/go.mod index 656810354b..0460f5a7d4 100644 --- a/sdk/go/go.mod +++ b/sdk/go/go.mod @@ -1,4 +1,4 @@ -module github.com/feast-dev/feast/sdk/go +module github.com/mrzzy/feast/sdk/go go 1.13 From b2a7ebc4eeb5303420cae77b64f905b9927bc7da Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Thu, 21 May 2020 09:58:56 +0800 Subject: [PATCH 06/13] Fix python sdk lint --- sdk/python/tests/test_client.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 9fe11ae9d3..e0286fdbec 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -15,7 +15,6 @@ import pkgutil import tempfile -from copy import deepcopy from concurrent import futures from datetime import datetime from unittest import mock @@ -252,9 +251,11 @@ def int_val(x): ) got_fields = got_response.field_values[0].fields - assert (got_fields["driver_id"] == int_val(1) - and got_fields["driver:age"] == int_val(1) - and got_fields["rating"] == int_val(9)) + assert ( + got_fields["driver_id"] == int_val(1) + and got_fields["driver:age"] == int_val(1) + and got_fields["rating"] == int_val(9) + ) @pytest.mark.parametrize( "mocked_client", From 564f5fe425d0aa8adaab627aed408a4412d34860 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Thu, 21 May 2020 11:06:35 +0800 Subject: [PATCH 07/13] Revert "Change repo referenced in go.mod to mrzzy fork to test modules retrieval" This reverts commit 13008ccef6b6cbee6299029589b5e9e864eee7ae. --- go.mod | 4 ++-- sdk/go/go.mod | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 557d4ddec5..1be2396f6e 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,10 @@ -module github.com/mrzzy/feast +module github.com/feast-dev/feast require ( github.com/Masterminds/goutils v1.1.0 // indirect github.com/Masterminds/semver v1.5.0 // indirect github.com/Masterminds/sprig v2.22.0+incompatible // indirect - github.com/mrzzy/feast/sdk/go v0.0.0-20200516052424-09ff3dda724c // indirect + github.com/feast-dev/feast/sdk/go v0.0.0-20200516052424-09ff3dda724c // indirect github.com/ghodss/yaml v1.0.0 github.com/gogo/protobuf v1.3.1 // indirect github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect diff --git a/sdk/go/go.mod b/sdk/go/go.mod index 0460f5a7d4..656810354b 100644 --- a/sdk/go/go.mod +++ b/sdk/go/go.mod @@ -1,4 +1,4 @@ -module github.com/mrzzy/feast/sdk/go +module github.com/feast-dev/feast/sdk/go go 1.13 From 9ad2f2e2bfdaede1348b3e0f60b70475905e90f7 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Thu, 21 May 2020 17:08:19 +0800 Subject: [PATCH 08/13] Optmisation: Init stripFields map to expected size in client.GetOnlineFeatures() --- sdk/go/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/go/client.go b/sdk/go/client.go index ef8ec8b8cf..1f0cbaa3b9 100644 --- a/sdk/go/client.go +++ b/sdk/go/client.go @@ -61,7 +61,7 @@ func (fc *GrpcClient) GetOnlineFeatures(ctx context.Context, req *OnlineFeatures // strip projects from to projects for _, fieldValue := range resp.GetFieldValues() { - stripFields := make(map[string]*types.Value) + stripFields := make(map[string]*types.Value, len(fieldValue.Fields)) for refStr, value := range fieldValue.Fields { _, isEntity := entityRefs[refStr] if !isEntity { // is feature ref From d7998303b0ef3f7815e1e71b519526d2f7935c1b Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Thu, 21 May 2020 17:10:50 +0800 Subject: [PATCH 09/13] Make the name of the client test case more descriptive --- sdk/go/client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/go/client_test.go b/sdk/go/client_test.go index a1902c500c..1ad6629e9a 100644 --- a/sdk/go/client_test.go +++ b/sdk/go/client_test.go @@ -22,7 +22,7 @@ func TestGetOnlineFeatures(t *testing.T) { err error }{ { - name: "valid", + name: "Valid client Get Online Features call", req: OnlineFeaturesRequest{ Features: []string{ "driver:rating", From eeeaceda7199035aab5e01464443000832b5f7e2 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 22 May 2020 16:47:13 +0800 Subject: [PATCH 10/13] Add missing mock files --- go.mod | 5 ++ go.sum | 6 +- sdk/go/mocks/serving_mock.go | 116 +++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 sdk/go/mocks/serving_mock.go diff --git a/go.mod b/go.mod index 1be2396f6e..b5a2539497 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,12 @@ require ( golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect golang.org/x/net v0.0.0-20200513185701-a91f0712d120 golang.org/x/sys v0.0.0-20200515095857-1151b9dac4a9 // indirect +<<<<<<< HEAD golang.org/x/tools v0.0.0-20200528185414-6be401e3f76e // indirect +======= + golang.org/x/tools v0.0.0-20200521211927-2b542361a4fc // indirect + google.golang.org/genproto v0.0.0-20200515170657-fc4c6c6a6587 // indirect +>>>>>>> Add missing mock files google.golang.org/grpc v1.29.1 google.golang.org/protobuf v1.24.0 // indirect gopkg.in/russross/blackfriday.v2 v2.0.0 // indirect diff --git a/go.sum b/go.sum index 1b2aa8bf63..90a517da7e 100644 --- a/go.sum +++ b/go.sum @@ -462,8 +462,10 @@ golang.org/x/tools v0.0.0-20200504022951-6b6965ac5dd1 h1:C8rdnd6KieI73Z2Av0sS0t4 golang.org/x/tools v0.0.0-20200504022951-6b6965ac5dd1/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200515220128-d3bf790afa53 h1:vmsb6v0zUdmUlXfwKaYrHPPRCV0lHq/IwNIf0ASGjyQ= golang.org/x/tools v0.0.0-20200515220128-d3bf790afa53/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= -golang.org/x/tools v0.0.0-20200528185414-6be401e3f76e h1:jTL1CJ2kmavapMVdBKy6oVrhBHByRCMfykS45+lEFQk= -golang.org/x/tools v0.0.0-20200528185414-6be401e3f76e/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= +golang.org/x/tools v0.0.0-20200519205726-57a9e4404bf7 h1:nm4zDh9WvH4jiuUpMY5RUsvOwrtTVVAsUaCdLW71hfY= +golang.org/x/tools v0.0.0-20200519205726-57a9e4404bf7/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= +golang.org/x/tools v0.0.0-20200521211927-2b542361a4fc h1:6m2YO+AmBApbUOmhsghW+IfRyZOY4My4UYvQQrEpHfY= +golang.org/x/tools v0.0.0-20200521211927-2b542361a4fc/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/sdk/go/mocks/serving_mock.go b/sdk/go/mocks/serving_mock.go new file mode 100644 index 0000000000..bea754c1bf --- /dev/null +++ b/sdk/go/mocks/serving_mock.go @@ -0,0 +1,116 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/feast-dev/feast/sdk/go/protos/feast/serving (interfaces: ServingServiceClient) + +// Package mock_serving is a generated GoMock package. +package mock_serving + +import ( + context "context" + serving "github.com/feast-dev/feast/sdk/go/protos/feast/serving" + gomock "github.com/golang/mock/gomock" + grpc "google.golang.org/grpc" + reflect "reflect" +) + +// MockServingServiceClient is a mock of ServingServiceClient interface +type MockServingServiceClient struct { + ctrl *gomock.Controller + recorder *MockServingServiceClientMockRecorder +} + +// MockServingServiceClientMockRecorder is the mock recorder for MockServingServiceClient +type MockServingServiceClientMockRecorder struct { + mock *MockServingServiceClient +} + +// NewMockServingServiceClient creates a new mock instance +func NewMockServingServiceClient(ctrl *gomock.Controller) *MockServingServiceClient { + mock := &MockServingServiceClient{ctrl: ctrl} + mock.recorder = &MockServingServiceClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockServingServiceClient) EXPECT() *MockServingServiceClientMockRecorder { + return m.recorder +} + +// GetBatchFeatures mocks base method +func (m *MockServingServiceClient) GetBatchFeatures(arg0 context.Context, arg1 *serving.GetBatchFeaturesRequest, arg2 ...grpc.CallOption) (*serving.GetBatchFeaturesResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetBatchFeatures", varargs...) + ret0, _ := ret[0].(*serving.GetBatchFeaturesResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetBatchFeatures indicates an expected call of GetBatchFeatures +func (mr *MockServingServiceClientMockRecorder) GetBatchFeatures(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBatchFeatures", reflect.TypeOf((*MockServingServiceClient)(nil).GetBatchFeatures), varargs...) +} + +// GetFeastServingInfo mocks base method +func (m *MockServingServiceClient) GetFeastServingInfo(arg0 context.Context, arg1 *serving.GetFeastServingInfoRequest, arg2 ...grpc.CallOption) (*serving.GetFeastServingInfoResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetFeastServingInfo", varargs...) + ret0, _ := ret[0].(*serving.GetFeastServingInfoResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetFeastServingInfo indicates an expected call of GetFeastServingInfo +func (mr *MockServingServiceClientMockRecorder) GetFeastServingInfo(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFeastServingInfo", reflect.TypeOf((*MockServingServiceClient)(nil).GetFeastServingInfo), varargs...) +} + +// GetJob mocks base method +func (m *MockServingServiceClient) GetJob(arg0 context.Context, arg1 *serving.GetJobRequest, arg2 ...grpc.CallOption) (*serving.GetJobResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetJob", varargs...) + ret0, _ := ret[0].(*serving.GetJobResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetJob indicates an expected call of GetJob +func (mr *MockServingServiceClientMockRecorder) GetJob(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetJob", reflect.TypeOf((*MockServingServiceClient)(nil).GetJob), varargs...) +} + +// GetOnlineFeatures mocks base method +func (m *MockServingServiceClient) GetOnlineFeatures(arg0 context.Context, arg1 *serving.GetOnlineFeaturesRequest, arg2 ...grpc.CallOption) (*serving.GetOnlineFeaturesResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetOnlineFeatures", varargs...) + ret0, _ := ret[0].(*serving.GetOnlineFeaturesResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetOnlineFeatures indicates an expected call of GetOnlineFeatures +func (mr *MockServingServiceClientMockRecorder) GetOnlineFeatures(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOnlineFeatures", reflect.TypeOf((*MockServingServiceClient)(nil).GetOnlineFeatures), varargs...) +} From adc60f23d9e065cfbb6cf9aed38c421ea327dc66 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 1 Jun 2020 11:24:06 +0800 Subject: [PATCH 11/13] Fix rebase markers in project root go mod --- go.mod | 4 ---- 1 file changed, 4 deletions(-) diff --git a/go.mod b/go.mod index b5a2539497..d5917cb33e 100644 --- a/go.mod +++ b/go.mod @@ -25,12 +25,8 @@ require ( golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect golang.org/x/net v0.0.0-20200513185701-a91f0712d120 golang.org/x/sys v0.0.0-20200515095857-1151b9dac4a9 // indirect -<<<<<<< HEAD - golang.org/x/tools v0.0.0-20200528185414-6be401e3f76e // indirect -======= golang.org/x/tools v0.0.0-20200521211927-2b542361a4fc // indirect google.golang.org/genproto v0.0.0-20200515170657-fc4c6c6a6587 // indirect ->>>>>>> Add missing mock files google.golang.org/grpc v1.29.1 google.golang.org/protobuf v1.24.0 // indirect gopkg.in/russross/blackfriday.v2 v2.0.0 // indirect From f1a79f4bb1fc06f4e19fa670bdae1e1e026cf890 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 1 Jun 2020 12:52:34 +0800 Subject: [PATCH 12/13] Fix compilation issue cause by downgrade to java 8 --- .../java/com/gojek/feast/FeastClientTest.java | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java b/sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java index f75a60832d..717792244e 100644 --- a/sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java +++ b/sdk/java/src/test/java/com/gojek/feast/FeastClientTest.java @@ -35,8 +35,9 @@ import io.grpc.stub.StreamObserver; import io.grpc.testing.GrpcCleanupRule; import java.time.Instant; +import java.util.Arrays; +import java.util.HashMap; import java.util.List; -import java.util.Map; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -85,17 +86,20 @@ public void setup() throws Exception { public void shouldGetOnlineFeatures() { List rows = this.client.getOnlineFeatures( - List.of("driver:name", "rating"), - List.of( + Arrays.asList("driver:name", "rating"), + Arrays.asList( Row.create().set("driver_id", 1).setEntityTimestamp(Instant.ofEpochSecond(100))), "driver_project"); assertEquals( rows.get(0).getFields(), - Map.of( - "driver_id", intValue(1), - "driver:name", strValue("david"), - "rating", intValue(3))); + new HashMap() { + { + put("driver_id", intValue(1)); + put("driver:name", strValue("david")); + put("rating", intValue(3)); + } + }); } private static GetOnlineFeaturesRequest getFakeRequest() { @@ -121,10 +125,13 @@ private static GetOnlineFeaturesResponse getFakeResponse() { .addFieldValues( FieldValues.newBuilder() .putAllFields( - Map.of( - "driver_id", intValue(1), - "driver:name", strValue("david"), - "rating", intValue(3))) + new HashMap() { + { + put("driver_id", intValue(1)); + put("driver:name", strValue("david")); + put("rating", intValue(3)); + } + }) .build()) .build(); } From 6f109ed80133404fa5baf0610321268e5a0a8a00 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 5 Jun 2020 11:53:56 +0800 Subject: [PATCH 13/13] Revert row count in python SDK client test to 300 --- sdk/python/tests/test_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index e0286fdbec..75d45a583f 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -202,7 +202,7 @@ def test_version(self, mocked_client, mocker): [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], ) def test_get_online_features(self, mocked_client, mocker): - ROW_COUNT = 1 + ROW_COUNT = 300 mocked_client._serving_service_stub = Serving.ServingServiceStub( grpc.insecure_channel("")