Skip to content

Commit

Permalink
Fix Go and Java SDK Regressions (feast-dev#729)
Browse files Browse the repository at this point in the history
* Fix nil error cause by  adding items to a nil map in go SDK's client.GetOnlineFeatures()

* Fix issue where go sdk client.GetOnlineFeatures() does not return entity values

* Fixed issue where Row is unable to parse Value type due to proto package rename.

* Fix wrong path of test_feature python SDK test.

* Revert stub and channel private fields to final in FeastClient java SDK

Co-authored-by: Zhu Zhanyan <zhu.zhanyan@gojek.com>
  • Loading branch information
2 people authored and khorshuheng committed Jun 5, 2020
1 parent 8bac240 commit b79274f
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 11 deletions.
8 changes: 4 additions & 4 deletions sdk/go/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (fc *GrpcClient) GetOnlineFeatures(ctx context.Context, req *OnlineFeatures
resp, err := fc.cli.GetOnlineFeatures(ctx, featuresRequest)

// collect unqiue entity refs from entity rows
var entityRefs map[string]struct{}
entityRefs := make(map[string]struct{})
for _, entityRows := range req.Entities {
for ref, _ := range entityRows {
entityRefs[ref] = struct{}{}
Expand All @@ -61,17 +61,17 @@ func (fc *GrpcClient) GetOnlineFeatures(ctx context.Context, req *OnlineFeatures

// strip projects from to projects
for _, fieldValue := range resp.GetFieldValues() {
var stripFields map[string]*types.Value
stripFields := make(map[string]*types.Value)
for refStr, value := range fieldValue.Fields {
_, isEntity := entityRefs[refStr]
if !isEntity { // is feature ref
featureRef, err := parseFeatureRef(refStr, true)
if err != nil {
return nil, err
}
stripRefStr := toFeatureRefStr(featureRef)
stripFields[stripRefStr] = value
refStr = toFeatureRefStr(featureRef)
}
stripFields[refStr] = value
}
fieldValue.Fields = stripFields
}
Expand Down
4 changes: 2 additions & 2 deletions sdk/go/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ func (r Row) equalTo(other Row) bool {
return true
}

// StrVal is a int64 type feast value
// StrVal is a string type feast value
func StrVal(val string) *types.Value {
return &types.Value{Val: &types.Value_StringVal{StringVal: val}}
}

// Int32Val is a int64 type feast value
// Int32Val is a int32 type feast value
func Int32Val(val int32) *types.Value {
return &types.Value{Val: &types.Value_Int32Val{Int32Val: val}}
}
Expand Down
7 changes: 4 additions & 3 deletions sdk/java/src/main/java/com/gojek/feast/FeastClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesRequest.EntityRow;
import feast.proto.serving.ServingAPIProto.GetOnlineFeaturesResponse;
import feast.proto.serving.ServingServiceGrpc;
import feast.proto.serving.ServingServiceGrpc.ServingServiceBlockingStub;
import feast.proto.types.ValueProto.Value;
import io.grpc.ManagedChannel;
import io.grpc.ManagedChannelBuilder;
Expand All @@ -40,7 +41,7 @@ public class FeastClient implements AutoCloseable {
private static final int CHANNEL_SHUTDOWN_TIMEOUT_SEC = 5;

private final ManagedChannel channel;
private final ServingServiceGrpc.ServingServiceBlockingStub stub;
private final ServingServiceBlockingStub stub;

/**
* Create a client to access Feast
Expand Down Expand Up @@ -161,9 +162,9 @@ public List<Row> getOnlineFeatures(
.collect(Collectors.toList());
}

private FeastClient(ManagedChannel channel) {
protected FeastClient(ManagedChannel channel) {
this.channel = channel;
stub = ServingServiceGrpc.newBlockingStub(channel);
this.stub = ServingServiceGrpc.newBlockingStub(channel);
}

public void close() throws Exception {
Expand Down
2 changes: 1 addition & 1 deletion sdk/java/src/main/java/com/gojek/feast/Row.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public Row set(String fieldName, Object value) {
fields.put(
fieldName, Value.newBuilder().setBytesVal(ByteString.copyFrom((byte[]) value)).build());
break;
case "feast.types.ValueProto.Value":
case "feast.proto.types.ValueProto.Value":
fields.put(fieldName, (Value) value);
break;
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

class TestFeatureRef:
def test_str_ref(self):
original_ref = FeatureRef(project="test", name="test")
original_ref = FeatureRef(feature_set="test", name="test")
ref_str = repr(original_ref)
parsed_ref = FeatureRef.from_str(ref_str)
assert original_ref == parsed_ref

0 comments on commit b79274f

Please sign in to comment.