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

Migrate SDKs to use Native types instead of Protobuf types #766

Closed
mrzzy opened this issue Jun 3, 2020 · 1 comment · Fixed by #826
Closed

Migrate SDKs to use Native types instead of Protobuf types #766

mrzzy opened this issue Jun 3, 2020 · 1 comment · Fixed by #826
Labels
compat/breaking Breaking user-facing change kind/techdebt

Comments

@mrzzy
Copy link
Collaborator

mrzzy commented Jun 3, 2020

Background

One of the stumbling blocks working on PR #658 with the SDKs is that the Python and Golang SDKs directly expose the Serving API to users. This is a problem changes with the Serving protobuf API will propagate to user API changes on the Python and Java client. This makes API compatibility hard to track and maintain. (see @ches's comment about it here)

Problem

Currently the SDKs expose internal Protobuf types directly to users.
For example:

This poses a problem as:

  • This makes the gRPC API exposed by Feast Serving/Core a user facing API, affecting downstream systems if changes are made to it.
  • Users prefer to deal with native types instead of Protobuf types. For example, Python users prefer to interact with datetime.timedelta instead of Protobuf's Duration type.

Proposed Solution

  1. Row API (similar to Java SDK’s Row) to be introduced to Python and Go SDKs as an higher level API.
  • ie Row would be used to abstract away GetOnlineFeaturesResponse etc.
feature_rows = client.get_online_features(....)
  1. Use native types in User Facing SDK APIs
  • ie:
func (fc *GrpcClient) GetFeastServingInfo(....) map[string]string
@mrzzy mrzzy changed the title Migrate SDKs to use Native Types Migrate SDKs to use NativeTypes instead of Protobuf types Jun 3, 2020
@mrzzy mrzzy changed the title Migrate SDKs to use NativeTypes instead of Protobuf types Migrate SDKs to use Native types instead of Protobuf types Jun 3, 2020
@woop
Copy link
Member

woop commented Jun 3, 2020

Thanks for this @mrzzy. I am assuming that this would also apply to EntityRows

    entity_rows=[
        GetOnlineFeaturesRequest.EntityRow(
            fields={
                "customer_id": Value(
                    int64_val=1
                )
            }
        )
    ],

Kind of nasty if the user just wants to set that 1. One option is the following

    entity_rows=[
        {"customer_id": 1001},
        {"customer_id": 1002},
        {"customer_id": 1003},
    ],

There would be a little bit of type inference required, but we can start with just 3 types (integer, float, string)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compat/breaking Breaking user-facing change kind/techdebt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants