Add Feature Tables API to Core & Python SDK #1019
Conversation
mrzzy
requested review from
davidheryanto,
khorshuheng,
pyalex,
woop and
zhilingc
as code owners
| } | ||
|
|
||
| message ListFeatureTablesRequest { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@woop will we support several online stores with different subscriptions in new architecture?
There was a problem hiding this comment.
So you mean we should support single project, wildcard feature table name matches?
There was a problem hiding this comment.
We only have a single online store. There is no such thing as a store subscription any more.
There was a problem hiding this comment.
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.
| protected static final Set<String> RESERVED_NAMES = | ||
| Set.of("created_timestamp", "event_timestamp"); | ||
|
|
||
| public static void validateSpec(FeatureTableSpec spec) { |
There was a problem hiding this comment.
does it check for empty features/entities list?
|
/retest |
1 similar comment
|
/retest |
| private String fieldMapJSON; | ||
|
|
||
| @Column(name = "ts_column") | ||
| private String tsColumn; |
There was a problem hiding this comment.
should we make this timestamp_column maybe?
There was a problem hiding this comment.
Replace all ts_column/tsColumn with timestamp_column/timestampColumn respectively.
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>
|
[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 |
|
/lgtm |
What this PR does / why we need it:
Implements the Feature Table API as defined in the 0.8 RFC
ApplyFeatureTable(),ListFeatureTables(),GetFeatureTable()apply(),list_feature_tables(),get_feature_table()(credits to @terryyylim).Does this PR introduce a user-facing change?: