Skip to content

Add Feature Tables API to Core & Python SDK#1019

Merged
feast-ci-bot merged 53 commits into
feast-dev:masterfrom
mrzzy:add-feature-tables
Oct 2, 2020
Merged

Add Feature Tables API to Core & Python SDK#1019
feast-ci-bot merged 53 commits into
feast-dev:masterfrom
mrzzy:add-feature-tables

Conversation

@mrzzy

@mrzzy mrzzy commented Sep 29, 2020

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:
Implements the Feature Table API as defined in the 0.8 RFC

  • Adds Feature Tables API to Core: ApplyFeatureTable(), ListFeatureTables(), GetFeatureTable()
  • Add Feature Tables support to Python SDK: apply(), list_feature_tables(), get_feature_table() (credits to @terryyylim).

Does this PR introduce a user-facing change?:

Add Feature Table apply, list, get to Feast Core and Python SDK.

Comment thread core/src/main/java/feast/core/grpc/CoreServiceImpl.java Outdated
Comment thread core/src/main/java/feast/core/model/FeatureSource.java Outdated
Comment thread core/src/main/java/feast/core/model/FeatureTable.java Outdated
Comment thread core/src/main/java/feast/core/model/FeatureTable.java Outdated
Comment thread core/src/main/java/feast/core/model/FeatureTable.java Outdated
Comment thread core/src/main/java/feast/core/model/FeatureTable.java Outdated
Comment thread core/src/main/java/feast/core/model/FeatureTable.java Outdated
Comment thread core/src/main/java/feast/core/service/SpecService.java Outdated
Comment thread core/src/main/java/feast/core/service/SpecService.java Outdated
Comment thread sdk/python/feast/feature_source.py Outdated
Comment thread sdk/python/feast/feature_source.py Outdated
Comment thread protos/feast/core/FeatureSource.proto Outdated
Comment thread core/src/main/java/feast/core/model/FeatureTable.java Outdated
Comment thread core/src/main/java/feast/core/service/SpecService.java Outdated
Comment thread protos/feast/core/CoreService.proto Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why we dropped wildcard search? it was used in JobController to retrieve all feature-sets by store subscription and I guess it could be useful for JobService as well if we won't drop subscription mechanism in stores.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@woop will we support several online stores with different subscriptions in new architecture?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So you mean we should support single project, wildcard feature table name matches?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We only have a single online store. There is no such thing as a store subscription any more.

@woop woop Sep 29, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When we implement a stream sink in the future, then we will need to create some kind of ingestion process that hooks up to a stream based on configuration to update an online store. But Feast doesnt know about these in the form of subscriptions. There is also no call to Feast Core to get this information.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

does it check for empty features/entities list?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added check in same file.

Comment thread core/src/test/java/feast/core/model/FeatureTableTest.java Outdated
@terryyylim

Copy link
Copy Markdown
Member

/retest

1 similar comment
@terryyylim

Copy link
Copy Markdown
Member

/retest

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we make this timestamp_column maybe?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Replace all ts_column/tsColumn with timestamp_column/timestampColumn respectively.

mrzzy and others added 22 commits October 2, 2020 08:53
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
…lds instead of just for feature.

Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
@feast-ci-bot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrzzy, woop

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@woop

woop commented Oct 2, 2020

Copy link
Copy Markdown
Member

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants