fix: Update on demand feature view api #2587

kevjumba · 2022-04-21T00:48:18Z

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

codecov-commenter · 2022-04-21T01:10:15Z

Codecov Report

Merging #2587 (99e0cf5) into master (ed2f979) will decrease coverage by 0.17%.
The diff coverage is 66.30%.

@@            Coverage Diff             @@
##           master    #2587      +/-   ##
==========================================
- Coverage   82.35%   82.18%   -0.18%     
==========================================
  Files         154      155       +1     
  Lines       12735    12854     +119     
==========================================
+ Hits        10488    10564      +76     
- Misses       2247     2290      +43

Flag	Coverage Δ
integrationtests	`72.11% <34.78%> (-0.89%)`	⬇️
unittests	`60.11% <53.26%> (+0.28%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...n/tests/integration/registration/test_inference.py	`100.00% <ø> (ø)`
...on/tests/integration/registration/test_registry.py	`95.87% <ø> (ø)`
sdk/python/feast/on_demand_feature_view.py	`81.42% <54.76%> (-4.80%)`	⬇️
...k/python/tests/unit/test_on_demand_feature_view.py	`68.75% <65.71%> (-6.25%)`	⬇️
sdk/python/feast/__init__.py	`92.00% <100.00%> (+0.69%)`	⬆️
sdk/python/feast/feature_view.py	`87.19% <100.00%> (+0.15%)`	⬆️
...ts/integration/feature_repos/repo_configuration.py	`89.14% <100.00%> (+0.06%)`	⬆️
...tegration/feature_repos/universal/feature_views.py	`81.94% <100.00%> (-1.39%)`	⬇️
...istration/test_universal_odfv_feature_inference.py	`100.00% <100.00%> (ø)`
sdk/python/feast/wait.py	`58.82% <0.00%> (-35.30%)`	⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed2f979...99e0cf5. Read the comment docs.

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

achals · 2022-04-21T18:59:58Z

docs/tutorials/validating-historical-features.md

@@ -134,7 +134,7 @@ taxi_entity = Entity(name='taxi', join_keys=['taxi_id'])


 ```python
-trips_stats_fv = FeatureView(
+trips_stats_fv = BaseFeatureView(


This is wrong, docs/user facing stuff should never refer to BaseFeatureView.

Ah thanks for the catch

achals · 2022-04-21T19:00:10Z

examples/java-demo/feature_repo/driver_repo.py


 driver_hourly_stats = FileSource(
    path="data/driver_stats_with_string.parquet",
    timestamp_field="event_timestamp",
    created_timestamp_column="created",
 )
 driver = Entity(name="driver_id", value_type=ValueType.INT64, description="driver id",)
-driver_hourly_stats_view = FeatureView(
+driver_hourly_stats_view = BaseFeatureView(


achals · 2022-04-21T19:01:28Z

sdk/python/feast/on_demand_feature_view.py

        self,
        *args,
        name: Optional[str] = None,
        features: Optional[List[Feature]] = None,
        sources: Optional[
-            Dict[str, Union[FeatureView, FeatureViewProjection, RequestSource]]
+            List[Union[BatchFeatureView, StreamFeatureView, RequestSource]]


We should probably continue to support FeatureViewProjection I think

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

sdk/python/feast/on_demand_feature_view.py

achals · 2022-04-22T17:27:19Z

sdk/python/feast/on_demand_feature_view.py

+                elif isinstance(source, FeatureViewProjection):
+                    _sources.append(
+                        BatchFeatureView(name=source.name, schema=source.features,)
+                    )


Hmmm I'm not sure exactly how projections are being used today, but i think it makes to continue to support them.

Oh I didn't push my changes I think, I updated this.

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

achals

/lgtm

achals · 2022-04-22T19:55:06Z

sdk/python/tests/integration/registration/test_registry.py

@@ -287,7 +287,7 @@ def odfv1(feature_df: pd.DataFrame) -> pd.DataFrame:
            Feature(name="odfv1_my_feature_1", dtype=ValueType.FLOAT),
            Feature(name="odfv1_my_feature_2", dtype=ValueType.INT32),
        ],
-        sources={"request_source": request_source},
+        sources=[request_source],


Do we have a test that tests the Dict[str, Source] version to make sure that the changes are backwards compatibile?

achals · 2022-04-22T19:55:56Z

sdk/python/tests/unit/test_on_demand_feature_view.py

+    with pytest.warns(DeprecationWarning):
+
+        @on_demand_feature_view(
+            inputs={"date_request": date_request},


oh hah never mind, you have this test. Love it.

feast-ci-bot · 2022-04-22T19:56:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: achals, kevjumba

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:

~~OWNERS~~ [achals,kevjumba]

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

# [0.21.0](v0.20.0...v0.21.0) (2022-05-13) ### Bug Fixes * Addresses ZeroDivisionError when materializing file source with same timestamps ([#2551](#2551)) ([1e398d9](1e398d9)) * Asynchronously refresh registry for the feast ui command ([#2672](#2672)) ([1b09ca2](1b09ca2)) * Build platform specific python packages with ci-build-wheel ([#2555](#2555)) ([b10a4cf](b10a4cf)) * Delete data sources from registry when using the diffing logic ([#2669](#2669)) ([fc00ca8](fc00ca8)) * Enforce kw args featureservice ([#2575](#2575)) ([160d7b7](160d7b7)) * Enforce kw args in datasources ([#2567](#2567)) ([0b7ec53](0b7ec53)) * Feature logging to Redshift is broken ([#2655](#2655)) ([479cd51](479cd51)) * Feature service to templates ([#2649](#2649)) ([1e02066](1e02066)) * Feature with timestamp type is incorrectly interpreted by Go FS ([#2588](#2588)) ([e3d9588](e3d9588)) * Fix `__hash__` methods ([#2556](#2556)) ([ebb7dfe](ebb7dfe)) * Fix AWS bootstrap template ([#2604](#2604)) ([c94a69c](c94a69c)) * Fix broken proto conversion methods for data sources ([#2603](#2603)) ([00ed65a](00ed65a)) * Fix case where on demand feature view tab is broken if no custom tabs are passed. ([#2682](#2682)) ([01d3568](01d3568)) * Fix DynamoDB fetches when there are entities that are not found ([#2573](#2573)) ([7076fe0](7076fe0)) * Fix Feast UI parser to work with new APIs ([#2668](#2668)) ([8d76751](8d76751)) * Fix java server after odfv update ([#2602](#2602)) ([0ca6297](0ca6297)) * Fix materialization with ttl=0 bug ([#2666](#2666)) ([ab78702](ab78702)) * Fix push sources and add docs / tests pushing via the python feature server ([#2561](#2561)) ([e8e418e](e8e418e)) * Fixed data mapping errors for Snowflake ([#2558](#2558)) ([53c2ce2](53c2ce2)) * Forcing ODFV udfs to be __main__ module and fixing false positive duplicate data source warning ([#2677](#2677)) ([2ce33cd](2ce33cd)) * Include the ui/build directory, and remove package data ([#2681](#2681)) ([0384f5f](0384f5f)) * Infer features for feature services when they depend on feature views without schemas ([#2653](#2653)) ([87c194c](87c194c)) * Pin dependencies to nearest major version ([#2647](#2647)) ([bb72b7c](bb72b7c)) * Pin pip<22.1 to get around breaking change in pip==22.1 ([#2678](#2678)) ([d3e01bc](d3e01bc)) * Punt deprecation warnings and clean up some warnings. ([#2670](#2670)) ([f775d2e](f775d2e)) * Reject undefined features when using `get_historical_features` or `get_online_features` ([#2665](#2665)) ([36849fb](36849fb)) * Remove ci extra from the feature transformation server dockerfile ([#2618](#2618)) ([25613b4](25613b4)) * Remove incorrect call to logging.basicConfig ([#2676](#2676)) ([8cbf51c](8cbf51c)) * Small typo in CLI ([#2578](#2578)) ([f372981](f372981)) * Switch from `join_key` to `join_keys` in tests and docs ([#2580](#2580)) ([d66c931](d66c931)) * Teardown trino container correctly after tests ([#2562](#2562)) ([72f1558](72f1558)) * Update build_go_protos to use a consistent python path ([#2550](#2550)) ([f136f8c](f136f8c)) * Update data source timestamp inference error message to make sense ([#2636](#2636)) ([3eaf6b7](3eaf6b7)) * Update field api to add tag parameter corresponding to labels in Feature. ([#2610](#2610)) ([689d20b](689d20b)) * Update java integration tests and add more logging ([#2637](#2637)) ([10e23b4](10e23b4)) * Update on demand feature view api ([#2587](#2587)) ([38cd7f9](38cd7f9)) * Update RedisCluster to use redis-py official implementation ([#2554](#2554)) ([ce5606f](ce5606f)) * Use cwd when getting module path ([#2577](#2577)) ([b550e59](b550e59)) * Use ParquetDataset for Schema Inference ([#2686](#2686)) ([4f85e3e](4f85e3e)) * Use timestamp type when converting unixtimestamp feature type to arrow ([#2593](#2593)) ([c439611](c439611)) ### Features * Add hbase online store support in feast ([#2590](#2590)) ([c9eda79](c9eda79)) * Adding SSL options for Postgres ([#2644](#2644)) ([0e809c2](0e809c2)) * Allow Feast UI to be spun up with CLI command: feast ui ([#2667](#2667)) ([44ca9f5](44ca9f5)) * Allow to pass secrets and environment variables to transformation service ([#2632](#2632)) ([ffa33ad](ffa33ad)) * CLI command 'feast serve' should start go-based server if flag is enabled ([#2617](#2617)) ([f3ff812](f3ff812)) * Create stream and batch feature view abstractions ([#2559](#2559)) ([d1f76e5](d1f76e5)) * Postgres supported as Registry, Online store, and Offline store ([#2401](#2401)) ([ed2f979](ed2f979)) * Support entity fields in feature view `schema` parameter by dropping them ([#2568](#2568)) ([c8fcc35](c8fcc35)) * Write logged features to an offline store (Python API) ([#2574](#2574)) ([134dc5f](134dc5f)) * Write logged features to Offline Store (Go - Python integration) ([#2621](#2621)) ([ccad832](ccad832)) ### Reverts * Revert "chore: Deprecate value type (#2611)" (#2643) ([4fbdfb1](4fbdfb1)), closes [#2611](#2611) [#2643](#2643)

kevjumba added 3 commits April 20, 2022 16:31

Migrate over

3235ce3

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix

27b7b6e

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix

6f01fd3

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

kevjumba added ok-to-test kind/housekeeping labels Apr 21, 2022

feast-ci-bot added the approved label Apr 21, 2022

kevjumba requested a review from achals April 21, 2022 00:48

feast-ci-bot added the size/L label Apr 21, 2022

kevjumba added 7 commits April 20, 2022 17:50

Fix

d77f9a1

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

temp fix

fb49cf3

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix lint

14fe923

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix lint

46eec61

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix lint

3742cf1

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix

f1f8f27

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix import

7ef6462

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

kevjumba assigned achals Apr 21, 2022

Fix

4c147ee

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

kevjumba added 6 commits April 21, 2022 10:28

Fix

e0078b1

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix lint

0b9d4fd

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix

390772c

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix lint

f6ab5da

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix integration tests

2047033

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix integration tests

4a2125f

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

achals reviewed Apr 22, 2022

View reviewed changes

kevjumba added 3 commits April 22, 2022 10:39

Fix

4b9dd34

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

lilnt

99e0cf5

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Fix lint?

e232ef2

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

achals reviewed Apr 22, 2022

View reviewed changes

Fix

05617db

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

achals approved these changes Apr 22, 2022

View reviewed changes

feast-ci-bot added the lgtm label Apr 22, 2022

feast-ci-bot merged commit 38cd7f9 into feast-dev:master Apr 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Update on demand feature view api #2587

fix: Update on demand feature view api #2587

kevjumba commented Apr 21, 2022

codecov-commenter commented Apr 21, 2022 •

edited

Loading

achals Apr 21, 2022

kevjumba Apr 22, 2022

achals Apr 21, 2022

kevjumba Apr 22, 2022

achals Apr 21, 2022

kevjumba Apr 22, 2022

achals Apr 22, 2022

kevjumba Apr 22, 2022

achals left a comment

achals Apr 22, 2022

achals Apr 22, 2022

feast-ci-bot commented Apr 22, 2022

fix: Update on demand feature view api #2587

fix: Update on demand feature view api #2587

Conversation

kevjumba commented Apr 21, 2022

codecov-commenter commented Apr 21, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achals left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feast-ci-bot commented Apr 22, 2022

codecov-commenter commented Apr 21, 2022 •

edited

Loading