Skip to content

Add support for feature set updates and remove versions#676

Merged
feast-ci-bot merged 19 commits intofeast-dev:masterfrom
zhilingc:version-removal-b
May 13, 2020
Merged

Add support for feature set updates and remove versions#676
feast-ci-bot merged 19 commits intofeast-dev:masterfrom
zhilingc:version-removal-b

Conversation

@zhilingc
Copy link
Copy Markdown
Collaborator

@zhilingc zhilingc commented May 4, 2020

What this PR does / why we need it:
Addresses #386, and implemented in accordance with this RFC.

For backward compatibility, user facing code still allows users to provide versions, but they will be ignored.

This PR is built on top of #655.

Which issue(s) this PR fixes:

Fixes #386

Does this PR introduce a user-facing change?:

Elements in the DB will have to be migrated to only retain the latest feature sets.

@feast-ci-bot
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zhilingc

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

@zhilingc zhilingc force-pushed the version-removal-b branch 6 times, most recently from d06868b to 37a2c6e Compare May 5, 2020 05:40
Copy link
Copy Markdown
Member

@woop woop left a comment

Choose a reason for hiding this comment

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

Is it possible for us to create integration/regression tests with the Python SDK that tests the feature addition or removal steps

  1. Read dataframe (filter a few columns)
  2. Infer a feature set and apply to Feast Core
  3. Ingest first half of dataframe (rows)
  4. Unfilter the previously filtered columns and filter other columns
  5. Infer a feature set and apply to Feast Core
  6. Ingest second half of dataframe (rows)
  7. Retrieve across all feature columns for complete history (should fail because of archival)
  8. Retrieve across current good feature columns for complete history (should return data over complete time range, but with gaps in the features that were filtered in step 1)

@zhilingc zhilingc force-pushed the version-removal-b branch 2 times, most recently from e991dc1 to 3617b33 Compare May 6, 2020 05:17
@woop woop changed the title Version removal, mutable feature sets Allow feature set updates and remove versions May 6, 2020
@woop woop added compat/breaking Breaking user-facing change kind/feature New feature or request labels May 6, 2020
@woop
Copy link
Copy Markdown
Member

woop commented May 6, 2020

Elements in the DB will have to be migrated to only retain the latest feature sets.

Should we add a note that versions will be ignored hence forth?

@woop
Copy link
Copy Markdown
Member

woop commented May 8, 2020

/retest

@zhilingc zhilingc force-pushed the version-removal-b branch from 04bf88b to e27e203 Compare May 13, 2020 06:31
@zhilingc zhilingc force-pushed the version-removal-b branch from 57144d3 to 2e9ff4e Compare May 13, 2020 09:29
@zhilingc zhilingc force-pushed the version-removal-b branch from 2e9ff4e to 6bbfede Compare May 13, 2020 09:54
@woop
Copy link
Copy Markdown
Member

woop commented May 13, 2020

/lgtm

@feast-ci-bot feast-ci-bot merged commit dfc81b9 into feast-dev:master May 13, 2020
string name = 2;

// Feature version
int32 version = 3;
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.

reserved for this one too?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature set versioning

6 participants