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

Make Projects optional & Update Feature References #693

Merged
merged 21 commits into from
May 19, 2020
Merged

Make Projects optional & Update Feature References #693

merged 21 commits into from
May 19, 2020

Conversation

mrzzy
Copy link
Collaborator

@mrzzy mrzzy commented May 12, 2020

What this PR does / why we need it:
Makes Projects optional by introducing a default project default:

  • Feast Core automatically creates a non archivable default project.
  • default project would be used where users do not explicit specify a project in CoreService's listFeatureSets(), getFeatureSet(), applyFeatureSet().
  • Feast Serving resolves Feature References without projects specified to Features in the default projects. The project field is now optional.

Update Feature References to support duplicate Features names within project but unique within Feature Set:

  • Introduces optional feature_set field to FeatureReference proto.
  • Allows users to specify feature set name by extending string Feature References to the format: [project/][feature_set:]feature_name with Feast Serving
  • Feast Serving attempts to resolve the feature references with missing project, and /or feature_set_name. If multiple features match, returns an error to mark the ambiguity.
  • Versions in Feature References are ignored by serving if specified.

When specifying Feature Set name in string feature references to get feature data (ie feature_set:feature)

  • Online Serving returns the feature data keyed by the submitted string feature reference.
  • Batch Serving returns the feature data in columns named in the format feature_set__feature

Serving attempts to maintain backwards compatibility with existing feature references in format: [project/]feature

Update Feast Go, Java, Python SDKs to transition to string Feature References without Projects:

  • Remove support for specifying projects in string feature references in SDKs. Specifying project in string Feature References supplied is now an error.
  • Add project field to get batch/online features methods to allow users to explicitly specify project.
  • String Feature References returned from get online features methods no longer contain project.

Which issue(s) this PR fixes:

Fixes #631

Does this PR introduce a user-facing change?:

Projects are now optional in Feast APIs (except ingestion).
- Feast creates a "default" project automatically.
- Feast uses the the "default" project where a project is not explicitly specified. 

Feast now namespaces Feature names within its Feature Set.
- Added optional "feature_set_name" field to FeatureReference proto.
- Feast supports FeatureSet specifying Feature Sets Name in string feature references: "driver_fs:driver_feature"
- Feast now allows features to have a unique name within a FeatureSet, but duplicated across a Project

Action Required: Dropping Support for specifying projects in string Feature References in Feast SDKs:
- Projects now cannot be specified in string Feature References passed to get online/batch features methods.
- Use the project parameter in those methods to specify projects instead. 
- String Feature References returned in get online features will not contain projects.

sdk/go/request.go Outdated Show resolved Hide resolved
sdk/go/request.go Outdated Show resolved Hide resolved
sdk/go/request.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@zhilingc zhilingc left a comment

Choose a reason for hiding this comment

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

I'll do a more thorough review once this has been rebased on top of version removal, since the code would be significantly easier to digest than in its current state.

core/src/main/java/feast/core/service/SpecService.java Outdated Show resolved Hide resolved
core/src/main/java/feast/core/service/SpecService.java Outdated Show resolved Hide resolved
core/src/main/java/feast/core/service/SpecService.java Outdated Show resolved Hide resolved
protos/feast/serving/ServingService.proto Outdated Show resolved Hide resolved
sdk/go/client.go Show resolved Hide resolved
@mrzzy
Copy link
Collaborator Author

mrzzy commented May 14, 2020

/test test-end-to-end-batch

@zhilingc
Copy link
Collaborator

/test test-end-to-end-redis-cluster

@mrzzy mrzzy changed the title WIP: Make Projects optional & Update Feature References Make Projects optional & Update Feature References May 16, 2020
@woop
Copy link
Member

woop commented May 16, 2020

/retest

1 similar comment
@woop
Copy link
Member

woop commented May 16, 2020

/retest

@mrzzy
Copy link
Collaborator Author

mrzzy commented May 16, 2020

/test test-end-to-end

@mrzzy
Copy link
Collaborator Author

mrzzy commented May 16, 2020

/test test-end-to-end-redis-cluster

if (request.getProject().isEmpty()) {
throw new IllegalArgumentException("No project provided");
request = request.toBuilder().setProject("default").build();
Copy link
Member

Choose a reason for hiding this comment

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

Should this be using one global project constant?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Corrected.

@mrzzy
Copy link
Collaborator Author

mrzzy commented May 18, 2020

/test test-end-to-end-batch

@woop
Copy link
Member

woop commented May 19, 2020

/test test-end-to-end-batch

@feast-ci-bot
Copy link
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

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
Copy link
Member

woop commented May 19, 2020

/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.

Validate feature reference and project functionality
4 participants