Executor build failures

[groszewn]

In attempting to rebuild the executor and prediction.proto to add custom datatype support, I came across a few different issues. I'll lay each out as followed so they can be addressed individually.

Proto compilation location

The output of compile_seldon_proto results in the compiled protobuf ending up under api/grpc/prediction.pb.go. The already existing version of this file falls under api/grpc/seldon/proto/prediction.pb.go, so I believe the Makefile may be placing this file in the wrong location.

Running make test after regenerating the seldon protobuf results in

go fmt ./...
can't load package: main.go:14:2: found packages grpc (client.go) and proto (prediction.pb.go) in /home/nicky/gitrepos/seldon-core/executor/api/grpc
make: *** [Makefile:14: fmt] Error 1

protoc-gen-go issues

Updating the location of the compiled protobuf to match what currently exists and then running make test results in the following issue.

# github.com/seldonio/seldon-core/executor/api/grpc/seldon/proto
api/grpc/seldon/proto/prediction.pb.go:802:11: undefined: grpc.SupportPackageIsVersion6
api/grpc/seldon/proto/prediction.pb.go:816:5: undefined: grpc.ClientConnInterface
make: *** [Makefile:18: vet] Error 2

This error comes from the version of protoc-gen-go installed. After version v1.3.2, the generatedCodeVersion has upgraded from 4 to 6. The version of protoc-gen-go can be pinned to v1.3.2 with the following

GIT_TAG="v1.3.2" # change as needed
go get -d -u github.com/golang/protobuf/protoc-gen-go
git -C "$(go env GOPATH)"/src/github.com/golang/protobuf checkout $GIT_TAG
go install github.com/golang/protobuf/protoc-gen-go

Even with protoc-gen-go pinned to the correct version, make test results in

go fmt ./...
go vet ./...
# github.com/seldonio/seldon-core/executor/api/util
api/util/utils.go:4:2: imported and not used: "github.com/seldonio/seldon-core/executor/api/grpc/seldon/proto" as api
api/util/utils.go:7:41: undefined: proto
api/util/utils.go:9:8: undefined: proto
api/util/utils.go:16:8: undefined: proto
api/util/utils.go:23:8: undefined: proto
# github.com/seldonio/seldon-core/executor/api/grpc/seldon/test
api/grpc/seldon/test/test_server.go:6:2: imported and not used: "github.com/seldonio/seldon-core/executor/api/grpc/seldon/proto" as api
api/grpc/seldon/test/test_server.go:23:65: undefined: proto
api/grpc/seldon/test/test_server.go:30:70: undefined: proto
# github.com/seldonio/seldon-core/executor/api/test
api/test/seldonmessage_test_client.go:5:2: imported and not used: "github.com/seldonio/seldon-core/executor/api/grpc/seldon/proto" as api
api/test/seldonmessage_test_client.go:47:16: undefined: proto
api/test/seldonmessage_test_client.go:82:56: undefined: proto
# github.com/seldonio/seldon-core/executor/api/payload
vet: api/payload/payload_test.go:11:9: undeclared name: proto
# github.com/seldonio/seldon-core/executor/api/grpc/tensorflow
vet: api/grpc/tensorflow/server_test.go:75:16: undeclared name: proto
make: *** [Makefile:18: vet] Error 2

Upon further inspection, this is due to the fact that the compiled protobuf has the wrong package name (api vs. the expected proto)

The latest stable release is now at v1.3.5, so I believe it will be best to stay up to date to make sure there aren't massive changes needed in the future to get back up to date. This update requires the version of grpc to be updated from 1.24.0 to 1.27.0 in order to work.

go_package inconsistencies

The compiled prediction protobuf that currently exists under the executor folder has a package name of proto. However, the go_package specified in the actual proto/prediction.proto is

option go_package = "github.com/seldonio/seldon-core/incubating/wrappers/s2i/go/pkg/api";

If you inspect the two separate compiled protobufs for go, you'll see that they differ:

1. Incubating golang wrapper compiled proto
2. Executor compiled proto

I'm not sure how the original compiled protobuf for the executor was compiled (without manual intervention), as no matter what I do, I end up with the package name being api instead of the expected proto.

Potential Changes

In order to remedy some of these issues, I'm thinking the following may be reasonable solutions:

• Use the latest protoc-gen-go version, which necessitates updating grpc from 1.24.0 to 1.27.0.
• Update the proto package name for the compiled seldon proto to match what's expected in some automated fashion. I'm not quite sure what the best way to handle this is, so suggestions are gladly welcomed :)

[groszewn]

The above merged PR should take care of this issue.