From d3e740b914aaea84cac328bc0458d4bdf8be078e Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 14 Jun 2022 16:31:45 -0700 Subject: [PATCH 01/26] Skaffolding for offline store push Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/offline_stores/offline_store.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/python/feast/infra/offline_stores/offline_store.py b/sdk/python/feast/infra/offline_stores/offline_store.py index cd807764ba8..2d2816c4e40 100644 --- a/sdk/python/feast/infra/offline_stores/offline_store.py +++ b/sdk/python/feast/infra/offline_stores/offline_store.py @@ -28,6 +28,8 @@ from feast.registry import BaseRegistry from feast.repo_config import RepoConfig from feast.saved_dataset import SavedDatasetStorage +from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto +from feast.protos.feast.types.Value_pb2 import Value as ValueProto if TYPE_CHECKING: from feast.saved_dataset import ValidationReference From 0b41400707ea0e270737d780d95f8340e5ce82fa Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 14 Jun 2022 16:33:14 -0700 Subject: [PATCH 02/26] LInt Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/offline_stores/offline_store.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/infra/offline_stores/offline_store.py b/sdk/python/feast/infra/offline_stores/offline_store.py index 2d2816c4e40..9c3f3f5ab49 100644 --- a/sdk/python/feast/infra/offline_stores/offline_store.py +++ b/sdk/python/feast/infra/offline_stores/offline_store.py @@ -25,11 +25,11 @@ from feast.feature_logging import LoggingConfig, LoggingSource from feast.feature_view import FeatureView from feast.on_demand_feature_view import OnDemandFeatureView +from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto +from feast.protos.feast.types.Value_pb2 import Value as ValueProto from feast.registry import BaseRegistry from feast.repo_config import RepoConfig from feast.saved_dataset import SavedDatasetStorage -from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto -from feast.protos.feast.types.Value_pb2 import Value as ValueProto if TYPE_CHECKING: from feast.saved_dataset import ValidationReference From b426888f8a8cefb6b40fadee5a2a999e2adf3bde Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 14 Jun 2022 16:55:26 -0700 Subject: [PATCH 03/26] Fix Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/offline_stores/offline_store.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/python/feast/infra/offline_stores/offline_store.py b/sdk/python/feast/infra/offline_stores/offline_store.py index 9c3f3f5ab49..cd807764ba8 100644 --- a/sdk/python/feast/infra/offline_stores/offline_store.py +++ b/sdk/python/feast/infra/offline_stores/offline_store.py @@ -25,8 +25,6 @@ from feast.feature_logging import LoggingConfig, LoggingSource from feast.feature_view import FeatureView from feast.on_demand_feature_view import OnDemandFeatureView -from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto -from feast.protos.feast.types.Value_pb2 import Value as ValueProto from feast.registry import BaseRegistry from feast.repo_config import RepoConfig from feast.saved_dataset import SavedDatasetStorage From 5187a198f9aa4e42abbfa2eca229116b667440c3 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Thu, 16 Jun 2022 14:07:54 -0700 Subject: [PATCH 04/26] File source offline push Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/offline_stores/file.py | 34 ++- .../feast/infra/passthrough_provider.py | 4 +- .../offline_store/test_offline_push.py | 196 ++++++++++++++++++ 3 files changed, 210 insertions(+), 24 deletions(-) create mode 100644 sdk/python/tests/integration/offline_store/test_offline_push.py diff --git a/sdk/python/feast/infra/offline_stores/file.py b/sdk/python/feast/infra/offline_stores/file.py index 194c233f53c..260f29bd88f 100644 --- a/sdk/python/feast/infra/offline_stores/file.py +++ b/sdk/python/feast/infra/offline_stores/file.py @@ -7,6 +7,7 @@ import pandas as pd import pyarrow import pyarrow.dataset +from pyarrow import csv import pyarrow.parquet import pytz from pydantic.typing import Literal @@ -405,42 +406,31 @@ def write_logged_features( ) @staticmethod - def offline_write_batch( - config: RepoConfig, - feature_view: FeatureView, - data: pyarrow.Table, - progress: Optional[Callable[[int], Any]], - ): + def offline_write_batch(config: RepoConfig, feature_view: FeatureView, data: pyarrow.Table, progress: Optional[Callable[[int], Any]]): if not feature_view.batch_source: - raise ValueError( - "feature view does not have a batch source to persist offline data" - ) + raise ValueError("feature view does not have a batch source to persist offline data") if not isinstance(config.offline_store, FileOfflineStoreConfig): - raise ValueError( - f"offline store config is of type {type(config.offline_store)} when file type required" - ) + raise ValueError(f"offline store config is of type {type(config.offline_store)} when file type required") if not isinstance(feature_view.batch_source, FileSource): - raise ValueError( - f"feature view batch source is {type(feature_view.batch_source)} not file source" - ) + raise ValueError(f"feature view batch source is {type(feature_view.batch_source)} not file source") file_options = feature_view.batch_source.file_options filesystem, path = FileSource.create_filesystem_and_path( file_options.uri, file_options.s3_endpoint_override ) prev_table = pyarrow.parquet.read_table(path, memory_map=True) - if prev_table.column_names != data.column_names: - raise ValueError( - f"Input dataframe has incorrect schema or wrong order, expected columns are: {prev_table.column_names}" - ) - if data.schema != prev_table.schema: + if(prev_table.column_names != data.column_names): + raise ValueError(f"Input dataframe have columns in wrong order, columns should be in the order: {prev_table.column_names}") + if(data.schema != prev_table.schema): data = data.cast(prev_table.schema) new_table = pyarrow.concat_tables([data, prev_table]) - writer = pyarrow.parquet.ParquetWriter(path, data.schema, filesystem=filesystem) + writer = pyarrow.parquet.ParquetWriter( + path, + data.schema, + filesystem=filesystem) writer.write_table(new_table) writer.close() - def _get_entity_df_event_timestamp_range( entity_df: Union[pd.DataFrame, str], entity_df_event_timestamp_col: str, ) -> Tuple[datetime, datetime]: diff --git a/sdk/python/feast/infra/passthrough_provider.py b/sdk/python/feast/infra/passthrough_provider.py index e023afe7829..9d18e6b249f 100644 --- a/sdk/python/feast/infra/passthrough_provider.py +++ b/sdk/python/feast/infra/passthrough_provider.py @@ -103,14 +103,14 @@ def online_write_batch( def offline_write_batch( self, config: RepoConfig, - table: FeatureView, + feature_view: FeatureView, data: pa.Table, progress: Optional[Callable[[int], Any]], ) -> None: set_usage_attribute("provider", self.__class__.__name__) if self.offline_store: - self.offline_store.offline_write_batch(config, table, data, progress) + self.offline_store.offline_write_batch(config, feature_view, data, progress) @log_exceptions_and_usage(sampler=RatioSampler(ratio=0.001)) def online_read( diff --git a/sdk/python/tests/integration/offline_store/test_offline_push.py b/sdk/python/tests/integration/offline_store/test_offline_push.py new file mode 100644 index 00000000000..d31a6ebf77a --- /dev/null +++ b/sdk/python/tests/integration/offline_store/test_offline_push.py @@ -0,0 +1,196 @@ + +import datetime +from datetime import datetime, timedelta + +import numpy as np +import pandas as pd +import pytest +import tempfile +import uuid + +from feast.data_format import ParquetFormat + +from feast import FeatureView, Field, FileSource +from feast.types import Int32, Float32 +from feast.wait import wait_retry_backoff +from tests.integration.feature_repos.repo_configuration import ( + construct_universal_feature_views, +) +from tests.integration.feature_repos.universal.data_sources.file import FileDataSourceCreator +from tests.integration.feature_repos.universal.entities import ( + customer, + driver, + location, +) +from tests.integration.feature_repos.universal.feature_views import conv_rate_plus_100 +from tests.utils.logged_features import prepare_logs, to_logs_dataset + +@pytest.mark.integration +@pytest.mark.universal_online_stores(only=["sqlite"]) +def test_writing_incorrect_order_fails(environment, universal_data_sources): + # TODO(kevjumba) handle incorrect order later, for now schema must be in the order that the filesource is in + store = environment.feature_store + _, _, data_sources = universal_data_sources + driver_stats = FeatureView( + name="driver_stats", + entities=["driver"], + schema=[ + Field(name="avg_daily_trips", dtype=Int32), + Field(name="conv_rate", dtype=Float32), + ], + source=data_sources.driver, + ) + + now = datetime.utcnow() + ts = pd.Timestamp(now).round("ms") + + entity_df = pd.DataFrame.from_dict( + { + "driver_id": [1001, 1002], + "event_timestamp": [ + ts-timedelta(hours=3), + ts, + ], + } + ) + + store.apply([driver(), driver_stats]) + df = store.get_historical_features( + entity_df=entity_df, + features=[ + "driver_stats:conv_rate", + "driver_stats:avg_daily_trips" + ], + full_feature_names=False, + ).to_df() + + assert df["conv_rate"].isnull().all() + assert df["avg_daily_trips"].isnull().all() + + expected_df = pd.DataFrame.from_dict( + { + "driver_id": [1001, 1002], + "event_timestamp": [ + ts-timedelta(hours=3), + ts, + ], + "conv_rate": [0.1, 0.2], + "avg_daily_trips": [1, 2], + "created": [ts, ts] + }, + ) + with pytest.raises(ValueError): + store.write_to_offline_store(driver_stats.name, expected_df, allow_registry_cache=False) + +@pytest.mark.integration +@pytest.mark.universal_online_stores(only=["sqlite"]) +def test_writing_consecutively_to_offline_store(environment, universal_data_sources): + store = environment.feature_store + _, _, data_sources = universal_data_sources + driver_stats = FeatureView( + name="driver_stats", + entities=["driver"], + schema=[ + Field(name="avg_daily_trips", dtype=Int32), + Field(name="conv_rate", dtype=Float32), + ], + source=data_sources.driver, + ttl=timedelta(minutes=10), + ) + + now = datetime.utcnow() + ts = pd.Timestamp(now, unit='ns') + + entity_df = pd.DataFrame.from_dict( + { + "driver_id": [1001, 1002], + "event_timestamp": [ + ts-timedelta(hours=4), + ts-timedelta(hours=3), + ], + } + ) + + store.apply([driver(), driver_stats]) + df = store.get_historical_features( + entity_df=entity_df, + features=[ + "driver_stats:conv_rate", + "driver_stats:avg_daily_trips" + ], + full_feature_names=False, + ).to_df() + + assert df["conv_rate"].isnull().all() + assert df["avg_daily_trips"].isnull().all() + + first_df = pd.DataFrame.from_dict( + { + "event_timestamp": [ + ts-timedelta(hours=4), + ts-timedelta(hours=3), + ], + "driver_id": [1001, 1001], + "conv_rate": [0.1, 0.2], + "acc_rate": [0.5, 0.6], + "avg_daily_trips": [1, 2], + "created": [ts, ts] + }, + ) + store.write_to_offline_store(driver_stats.name, first_df, allow_registry_cache=False) + + after_write_df = store.get_historical_features( + entity_df=entity_df, + features=[ + "driver_stats:conv_rate", + "driver_stats:avg_daily_trips" + ], + full_feature_names=False, + ).to_df() + + assert len(after_write_df) == len(first_df) + assert np.where(after_write_df["conv_rate"].reset_index(drop=True) == first_df["conv_rate"].reset_index(drop=True)) + assert np.where(after_write_df["avg_daily_trips"].reset_index(drop=True) == first_df["avg_daily_trips"].reset_index(drop=True)) + + second_df = pd.DataFrame.from_dict( + { + "event_timestamp": [ + ts-timedelta(hours=1), + ts, + ], + "driver_id": [1001, 1001], + "conv_rate": [0.3, 0.4], + "acc_rate": [0.8, 0.9], + "avg_daily_trips": [3, 4], + "created": [ts, ts] + }, + ) + + store.write_to_offline_store(driver_stats.name, second_df, allow_registry_cache=False) + + entity_df = pd.DataFrame.from_dict( + { + "driver_id": [1001, 1001, 1001, 1001], + "event_timestamp": [ + ts-timedelta(hours=4), + ts-timedelta(hours=3), + ts-timedelta(hours=1), + ts, + ], + } + ) + + after_write_df = store.get_historical_features( + entity_df=entity_df, + features=[ + "driver_stats:conv_rate", + "driver_stats:avg_daily_trips" + ], + full_feature_names=False, + ).to_df() + + expected_df = pd.concat([first_df, second_df]) + assert len(after_write_df) == len(expected_df) + assert np.where(after_write_df["conv_rate"].reset_index(drop=True) == expected_df["conv_rate"].reset_index(drop=True)) + assert np.where(after_write_df["avg_daily_trips"].reset_index(drop=True) == expected_df["avg_daily_trips"].reset_index(drop=True)) + From 42133bdb53a4b60689bdc769f9d229697170c896 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Thu, 16 Jun 2022 15:38:26 -0700 Subject: [PATCH 05/26] Fix Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/offline_stores/file.py | 2 +- .../offline_store/test_offline_push.py | 79 ++++++++++++++++--- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/sdk/python/feast/infra/offline_stores/file.py b/sdk/python/feast/infra/offline_stores/file.py index 260f29bd88f..85028c9236b 100644 --- a/sdk/python/feast/infra/offline_stores/file.py +++ b/sdk/python/feast/infra/offline_stores/file.py @@ -420,7 +420,7 @@ def offline_write_batch(config: RepoConfig, feature_view: FeatureView, data: pya prev_table = pyarrow.parquet.read_table(path, memory_map=True) if(prev_table.column_names != data.column_names): - raise ValueError(f"Input dataframe have columns in wrong order, columns should be in the order: {prev_table.column_names}") + raise ValueError(f"Input dataframe has incorrect schema or wrong order, expected columns are: {prev_table.column_names}") if(data.schema != prev_table.schema): data = data.cast(prev_table.schema) new_table = pyarrow.concat_tables([data, prev_table]) diff --git a/sdk/python/tests/integration/offline_store/test_offline_push.py b/sdk/python/tests/integration/offline_store/test_offline_push.py index d31a6ebf77a..4b6fb557f49 100644 --- a/sdk/python/tests/integration/offline_store/test_offline_push.py +++ b/sdk/python/tests/integration/offline_store/test_offline_push.py @@ -5,8 +5,7 @@ import numpy as np import pandas as pd import pytest -import tempfile -import uuid +import random from feast.data_format import ParquetFormat @@ -74,8 +73,66 @@ def test_writing_incorrect_order_fails(environment, universal_data_sources): ts-timedelta(hours=3), ts, ], - "conv_rate": [0.1, 0.2], - "avg_daily_trips": [1, 2], + "conv_rate": [random.random(), random.random()], + "avg_daily_trips": [random.randint(0, 10), random.randint(0, 10)], + "created": [ts, ts] + }, + ) + with pytest.raises(ValueError): + store.write_to_offline_store(driver_stats.name, expected_df, allow_registry_cache=False) + + +@pytest.mark.integration +@pytest.mark.universal_online_stores(only=["sqlite"]) +def test_writing_incorrect_schema_fails(environment, universal_data_sources): + # TODO(kevjumba) handle incorrect order later, for now schema must be in the order that the filesource is in + store = environment.feature_store + _, _, data_sources = universal_data_sources + driver_stats = FeatureView( + name="driver_stats", + entities=["driver"], + schema=[ + Field(name="avg_daily_trips", dtype=Int32), + Field(name="conv_rate", dtype=Float32), + ], + source=data_sources.driver, + ) + + now = datetime.utcnow() + ts = pd.Timestamp(now).round("ms") + + entity_df = pd.DataFrame.from_dict( + { + "driver_id": [1001, 1002], + "event_timestamp": [ + ts-timedelta(hours=3), + ts, + ], + } + ) + + store.apply([driver(), driver_stats]) + df = store.get_historical_features( + entity_df=entity_df, + features=[ + "driver_stats:conv_rate", + "driver_stats:avg_daily_trips" + ], + full_feature_names=False, + ).to_df() + + assert df["conv_rate"].isnull().all() + assert df["avg_daily_trips"].isnull().all() + + expected_df = pd.DataFrame.from_dict( + { + "event_timestamp": [ + ts-timedelta(hours=3), + ts, + ], + "driver_id": [1001, 1002], + "conv_rate": [random.random(), random.random()], + "incorrect_schema": [random.randint(0, 10), random.randint(0, 10)], "created": [ts, ts] }, ) @@ -103,7 +160,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour entity_df = pd.DataFrame.from_dict( { - "driver_id": [1001, 1002], + "driver_id": [1001, 1001], "event_timestamp": [ ts-timedelta(hours=4), ts-timedelta(hours=3), @@ -131,9 +188,9 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour ts-timedelta(hours=3), ], "driver_id": [1001, 1001], - "conv_rate": [0.1, 0.2], - "acc_rate": [0.5, 0.6], - "avg_daily_trips": [1, 2], + "conv_rate": [random.random(), random.random()], + "acc_rate": [random.random(), random.random()], + "avg_daily_trips": [random.randint(0, 10), random.randint(0, 10)], "created": [ts, ts] }, ) @@ -159,9 +216,9 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour ts, ], "driver_id": [1001, 1001], - "conv_rate": [0.3, 0.4], - "acc_rate": [0.8, 0.9], - "avg_daily_trips": [3, 4], + "conv_rate": [random.random(), random.random()], + "acc_rate": [random.random(), random.random()], + "avg_daily_trips": [random.randint(0, 10), random.randint(0, 10)], "created": [ts, ts] }, ) From cd45c2c9fa64ac2c11fde6b1047da412495e45e9 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Thu, 16 Jun 2022 15:55:55 -0700 Subject: [PATCH 06/26] Fix Signed-off-by: Kevin Zhang --- .../integration/offline_store/test_offline_push.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/sdk/python/tests/integration/offline_store/test_offline_push.py b/sdk/python/tests/integration/offline_store/test_offline_push.py index 4b6fb557f49..85adc542fc7 100644 --- a/sdk/python/tests/integration/offline_store/test_offline_push.py +++ b/sdk/python/tests/integration/offline_store/test_offline_push.py @@ -25,7 +25,7 @@ from tests.utils.logged_features import prepare_logs, to_logs_dataset @pytest.mark.integration -@pytest.mark.universal_online_stores(only=["sqlite"]) +@pytest.mark.universal_online_stores def test_writing_incorrect_order_fails(environment, universal_data_sources): # TODO(kevjumba) handle incorrect order later, for now schema must be in the order that the filesource is in store = environment.feature_store @@ -83,7 +83,7 @@ def test_writing_incorrect_order_fails(environment, universal_data_sources): @pytest.mark.integration -@pytest.mark.universal_online_stores(only=["sqlite"]) +@pytest.mark.universal_online_stores def test_writing_incorrect_schema_fails(environment, universal_data_sources): # TODO(kevjumba) handle incorrect order later, for now schema must be in the order that the filesource is in store = environment.feature_store @@ -140,7 +140,7 @@ def test_writing_incorrect_schema_fails(environment, universal_data_sources): store.write_to_offline_store(driver_stats.name, expected_df, allow_registry_cache=False) @pytest.mark.integration -@pytest.mark.universal_online_stores(only=["sqlite"]) +@pytest.mark.universal_online_stores def test_writing_consecutively_to_offline_store(environment, universal_data_sources): store = environment.feature_store _, _, data_sources = universal_data_sources @@ -150,6 +150,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour schema=[ Field(name="avg_daily_trips", dtype=Int32), Field(name="conv_rate", dtype=Float32), + Field(name="acc_rate", dtype=Float32), ], source=data_sources.driver, ttl=timedelta(minutes=10), @@ -173,6 +174,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour entity_df=entity_df, features=[ "driver_stats:conv_rate", + "driver_stats:avg_daily_trips" ], full_feature_names=False, @@ -241,6 +243,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour entity_df=entity_df, features=[ "driver_stats:conv_rate", + "driver_stats:acc_rate", "driver_stats:avg_daily_trips" ], full_feature_names=False, @@ -249,5 +252,5 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour expected_df = pd.concat([first_df, second_df]) assert len(after_write_df) == len(expected_df) assert np.where(after_write_df["conv_rate"].reset_index(drop=True) == expected_df["conv_rate"].reset_index(drop=True)) + assert np.where(after_write_df["acc_rate"].reset_index(drop=True) == expected_df["acc_rate"].reset_index(drop=True)) assert np.where(after_write_df["avg_daily_trips"].reset_index(drop=True) == expected_df["avg_daily_trips"].reset_index(drop=True)) - From 0a62346cc09e4da3e6342d434decc43f316d363c Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Thu, 16 Jun 2022 15:58:34 -0700 Subject: [PATCH 07/26] Fix Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/offline_stores/file.py | 16 +- .../offline_store/test_offline_push.py | 141 +++++++----------- 2 files changed, 65 insertions(+), 92 deletions(-) diff --git a/sdk/python/feast/infra/offline_stores/file.py b/sdk/python/feast/infra/offline_stores/file.py index 85028c9236b..7856eaa1c9a 100644 --- a/sdk/python/feast/infra/offline_stores/file.py +++ b/sdk/python/feast/infra/offline_stores/file.py @@ -7,9 +7,9 @@ import pandas as pd import pyarrow import pyarrow.dataset -from pyarrow import csv import pyarrow.parquet import pytz +from pyarrow import csv from pydantic.typing import Literal from feast import FileSource, OnDemandFeatureView @@ -419,18 +419,18 @@ def offline_write_batch(config: RepoConfig, feature_view: FeatureView, data: pya ) prev_table = pyarrow.parquet.read_table(path, memory_map=True) - if(prev_table.column_names != data.column_names): - raise ValueError(f"Input dataframe has incorrect schema or wrong order, expected columns are: {prev_table.column_names}") - if(data.schema != prev_table.schema): + if prev_table.column_names != data.column_names: + raise ValueError( + f"Input dataframe has incorrect schema or wrong order, expected columns are: {prev_table.column_names}" + ) + if data.schema != prev_table.schema: data = data.cast(prev_table.schema) new_table = pyarrow.concat_tables([data, prev_table]) - writer = pyarrow.parquet.ParquetWriter( - path, - data.schema, - filesystem=filesystem) + writer = pyarrow.parquet.ParquetWriter(path, data.schema, filesystem=filesystem) writer.write_table(new_table) writer.close() + def _get_entity_df_event_timestamp_range( entity_df: Union[pd.DataFrame, str], entity_df_event_timestamp_col: str, ) -> Tuple[datetime, datetime]: diff --git a/sdk/python/tests/integration/offline_store/test_offline_push.py b/sdk/python/tests/integration/offline_store/test_offline_push.py index 85adc542fc7..ba851e2918d 100644 --- a/sdk/python/tests/integration/offline_store/test_offline_push.py +++ b/sdk/python/tests/integration/offline_store/test_offline_push.py @@ -1,28 +1,17 @@ -import datetime +import random from datetime import datetime, timedelta import numpy as np import pandas as pd import pytest -import random -from feast.data_format import ParquetFormat - -from feast import FeatureView, Field, FileSource -from feast.types import Int32, Float32 -from feast.wait import wait_retry_backoff -from tests.integration.feature_repos.repo_configuration import ( - construct_universal_feature_views, -) -from tests.integration.feature_repos.universal.data_sources.file import FileDataSourceCreator +from feast import FeatureView, Field +from feast.types import Float32, Int32 from tests.integration.feature_repos.universal.entities import ( - customer, driver, - location, ) -from tests.integration.feature_repos.universal.feature_views import conv_rate_plus_100 -from tests.utils.logged_features import prepare_logs, to_logs_dataset + @pytest.mark.integration @pytest.mark.universal_online_stores @@ -44,22 +33,13 @@ def test_writing_incorrect_order_fails(environment, universal_data_sources): ts = pd.Timestamp(now).round("ms") entity_df = pd.DataFrame.from_dict( - { - "driver_id": [1001, 1002], - "event_timestamp": [ - ts-timedelta(hours=3), - ts, - ], - } + {"driver_id": [1001, 1002], "event_timestamp": [ts - timedelta(hours=3), ts,],} ) store.apply([driver(), driver_stats]) df = store.get_historical_features( entity_df=entity_df, - features=[ - "driver_stats:conv_rate", - "driver_stats:avg_daily_trips" - ], + features=["driver_stats:conv_rate", "driver_stats:avg_daily_trips"], full_feature_names=False, ).to_df() @@ -69,17 +49,16 @@ def test_writing_incorrect_order_fails(environment, universal_data_sources): expected_df = pd.DataFrame.from_dict( { "driver_id": [1001, 1002], - "event_timestamp": [ - ts-timedelta(hours=3), - ts, - ], + "event_timestamp": [ts - timedelta(hours=3), ts,], "conv_rate": [random.random(), random.random()], "avg_daily_trips": [random.randint(0, 10), random.randint(0, 10)], - "created": [ts, ts] + "created": [ts, ts], }, ) with pytest.raises(ValueError): - store.write_to_offline_store(driver_stats.name, expected_df, allow_registry_cache=False) + store.write_to_offline_store( + driver_stats.name, expected_df, allow_registry_cache=False + ) @pytest.mark.integration @@ -102,22 +81,13 @@ def test_writing_incorrect_schema_fails(environment, universal_data_sources): ts = pd.Timestamp(now).round("ms") entity_df = pd.DataFrame.from_dict( - { - "driver_id": [1001, 1002], - "event_timestamp": [ - ts-timedelta(hours=3), - ts, - ], - } + {"driver_id": [1001, 1002], "event_timestamp": [ts - timedelta(hours=3), ts,],} ) store.apply([driver(), driver_stats]) df = store.get_historical_features( entity_df=entity_df, - features=[ - "driver_stats:conv_rate", - "driver_stats:avg_daily_trips" - ], + features=["driver_stats:conv_rate", "driver_stats:avg_daily_trips"], full_feature_names=False, ).to_df() @@ -126,18 +96,18 @@ def test_writing_incorrect_schema_fails(environment, universal_data_sources): expected_df = pd.DataFrame.from_dict( { - "event_timestamp": [ - ts-timedelta(hours=3), - ts, - ], + "event_timestamp": [ts - timedelta(hours=3), ts,], "driver_id": [1001, 1002], "conv_rate": [random.random(), random.random()], "incorrect_schema": [random.randint(0, 10), random.randint(0, 10)], - "created": [ts, ts] + "created": [ts, ts], }, ) with pytest.raises(ValueError): - store.write_to_offline_store(driver_stats.name, expected_df, allow_registry_cache=False) + store.write_to_offline_store( + driver_stats.name, expected_df, allow_registry_cache=False + ) + @pytest.mark.integration @pytest.mark.universal_online_stores @@ -157,26 +127,19 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour ) now = datetime.utcnow() - ts = pd.Timestamp(now, unit='ns') + ts = pd.Timestamp(now, unit="ns") entity_df = pd.DataFrame.from_dict( { "driver_id": [1001, 1001], - "event_timestamp": [ - ts-timedelta(hours=4), - ts-timedelta(hours=3), - ], + "event_timestamp": [ts - timedelta(hours=4), ts - timedelta(hours=3),], } ) store.apply([driver(), driver_stats]) df = store.get_historical_features( entity_df=entity_df, - features=[ - "driver_stats:conv_rate", - - "driver_stats:avg_daily_trips" - ], + features=["driver_stats:conv_rate", "driver_stats:avg_daily_trips"], full_feature_names=False, ).to_df() @@ -185,55 +148,56 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour first_df = pd.DataFrame.from_dict( { - "event_timestamp": [ - ts-timedelta(hours=4), - ts-timedelta(hours=3), - ], + "event_timestamp": [ts - timedelta(hours=4), ts - timedelta(hours=3),], "driver_id": [1001, 1001], "conv_rate": [random.random(), random.random()], "acc_rate": [random.random(), random.random()], "avg_daily_trips": [random.randint(0, 10), random.randint(0, 10)], - "created": [ts, ts] + "created": [ts, ts], }, ) - store.write_to_offline_store(driver_stats.name, first_df, allow_registry_cache=False) + store.write_to_offline_store( + driver_stats.name, first_df, allow_registry_cache=False + ) after_write_df = store.get_historical_features( entity_df=entity_df, - features=[ - "driver_stats:conv_rate", - "driver_stats:avg_daily_trips" - ], + features=["driver_stats:conv_rate", "driver_stats:avg_daily_trips"], full_feature_names=False, ).to_df() assert len(after_write_df) == len(first_df) - assert np.where(after_write_df["conv_rate"].reset_index(drop=True) == first_df["conv_rate"].reset_index(drop=True)) - assert np.where(after_write_df["avg_daily_trips"].reset_index(drop=True) == first_df["avg_daily_trips"].reset_index(drop=True)) + assert np.where( + after_write_df["conv_rate"].reset_index(drop=True) + == first_df["conv_rate"].reset_index(drop=True) + ) + assert np.where( + after_write_df["avg_daily_trips"].reset_index(drop=True) + == first_df["avg_daily_trips"].reset_index(drop=True) + ) second_df = pd.DataFrame.from_dict( { - "event_timestamp": [ - ts-timedelta(hours=1), - ts, - ], + "event_timestamp": [ts - timedelta(hours=1), ts,], "driver_id": [1001, 1001], "conv_rate": [random.random(), random.random()], "acc_rate": [random.random(), random.random()], "avg_daily_trips": [random.randint(0, 10), random.randint(0, 10)], - "created": [ts, ts] + "created": [ts, ts], }, ) - store.write_to_offline_store(driver_stats.name, second_df, allow_registry_cache=False) + store.write_to_offline_store( + driver_stats.name, second_df, allow_registry_cache=False + ) entity_df = pd.DataFrame.from_dict( { "driver_id": [1001, 1001, 1001, 1001], "event_timestamp": [ - ts-timedelta(hours=4), - ts-timedelta(hours=3), - ts-timedelta(hours=1), + ts - timedelta(hours=4), + ts - timedelta(hours=3), + ts - timedelta(hours=1), ts, ], } @@ -244,13 +208,22 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour features=[ "driver_stats:conv_rate", "driver_stats:acc_rate", - "driver_stats:avg_daily_trips" + "driver_stats:avg_daily_trips", ], full_feature_names=False, ).to_df() expected_df = pd.concat([first_df, second_df]) assert len(after_write_df) == len(expected_df) - assert np.where(after_write_df["conv_rate"].reset_index(drop=True) == expected_df["conv_rate"].reset_index(drop=True)) - assert np.where(after_write_df["acc_rate"].reset_index(drop=True) == expected_df["acc_rate"].reset_index(drop=True)) - assert np.where(after_write_df["avg_daily_trips"].reset_index(drop=True) == expected_df["avg_daily_trips"].reset_index(drop=True)) + assert np.where( + after_write_df["conv_rate"].reset_index(drop=True) + == expected_df["conv_rate"].reset_index(drop=True) + ) + assert np.where( + after_write_df["acc_rate"].reset_index(drop=True) + == expected_df["acc_rate"].reset_index(drop=True) + ) + assert np.where( + after_write_df["avg_daily_trips"].reset_index(drop=True) + == expected_df["avg_daily_trips"].reset_index(drop=True) + ) From e022dc45a1b8048118302793bbccdeb3ba643856 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Thu, 16 Jun 2022 16:03:18 -0700 Subject: [PATCH 08/26] Fix Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/offline_stores/file.py | 1 - .../offline_store/test_offline_push.py | 19 ++++++++----------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/sdk/python/feast/infra/offline_stores/file.py b/sdk/python/feast/infra/offline_stores/file.py index 7856eaa1c9a..b0bf94e3522 100644 --- a/sdk/python/feast/infra/offline_stores/file.py +++ b/sdk/python/feast/infra/offline_stores/file.py @@ -9,7 +9,6 @@ import pyarrow.dataset import pyarrow.parquet import pytz -from pyarrow import csv from pydantic.typing import Literal from feast import FileSource, OnDemandFeatureView diff --git a/sdk/python/tests/integration/offline_store/test_offline_push.py b/sdk/python/tests/integration/offline_store/test_offline_push.py index ba851e2918d..068b7b0a754 100644 --- a/sdk/python/tests/integration/offline_store/test_offline_push.py +++ b/sdk/python/tests/integration/offline_store/test_offline_push.py @@ -1,4 +1,3 @@ - import random from datetime import datetime, timedelta @@ -8,9 +7,7 @@ from feast import FeatureView, Field from feast.types import Float32, Int32 -from tests.integration.feature_repos.universal.entities import ( - driver, -) +from tests.integration.feature_repos.universal.entities import driver @pytest.mark.integration @@ -33,7 +30,7 @@ def test_writing_incorrect_order_fails(environment, universal_data_sources): ts = pd.Timestamp(now).round("ms") entity_df = pd.DataFrame.from_dict( - {"driver_id": [1001, 1002], "event_timestamp": [ts - timedelta(hours=3), ts,],} + {"driver_id": [1001, 1002], "event_timestamp": [ts - timedelta(hours=3), ts]} ) store.apply([driver(), driver_stats]) @@ -49,7 +46,7 @@ def test_writing_incorrect_order_fails(environment, universal_data_sources): expected_df = pd.DataFrame.from_dict( { "driver_id": [1001, 1002], - "event_timestamp": [ts - timedelta(hours=3), ts,], + "event_timestamp": [ts - timedelta(hours=3), ts], "conv_rate": [random.random(), random.random()], "avg_daily_trips": [random.randint(0, 10), random.randint(0, 10)], "created": [ts, ts], @@ -81,7 +78,7 @@ def test_writing_incorrect_schema_fails(environment, universal_data_sources): ts = pd.Timestamp(now).round("ms") entity_df = pd.DataFrame.from_dict( - {"driver_id": [1001, 1002], "event_timestamp": [ts - timedelta(hours=3), ts,],} + {"driver_id": [1001, 1002], "event_timestamp": [ts - timedelta(hours=3), ts]} ) store.apply([driver(), driver_stats]) @@ -96,7 +93,7 @@ def test_writing_incorrect_schema_fails(environment, universal_data_sources): expected_df = pd.DataFrame.from_dict( { - "event_timestamp": [ts - timedelta(hours=3), ts,], + "event_timestamp": [ts - timedelta(hours=3), ts], "driver_id": [1001, 1002], "conv_rate": [random.random(), random.random()], "incorrect_schema": [random.randint(0, 10), random.randint(0, 10)], @@ -132,7 +129,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour entity_df = pd.DataFrame.from_dict( { "driver_id": [1001, 1001], - "event_timestamp": [ts - timedelta(hours=4), ts - timedelta(hours=3),], + "event_timestamp": [ts - timedelta(hours=4), ts - timedelta(hours=3)], } ) @@ -148,7 +145,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour first_df = pd.DataFrame.from_dict( { - "event_timestamp": [ts - timedelta(hours=4), ts - timedelta(hours=3),], + "event_timestamp": [ts - timedelta(hours=4), ts - timedelta(hours=3)], "driver_id": [1001, 1001], "conv_rate": [random.random(), random.random()], "acc_rate": [random.random(), random.random()], @@ -178,7 +175,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour second_df = pd.DataFrame.from_dict( { - "event_timestamp": [ts - timedelta(hours=1), ts,], + "event_timestamp": [ts - timedelta(hours=1), ts], "driver_id": [1001, 1001], "conv_rate": [random.random(), random.random()], "acc_rate": [random.random(), random.random()], From 5e0a699f6baee93de05cb9248c4fc7445e2c842a Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Thu, 16 Jun 2022 18:12:36 -0700 Subject: [PATCH 09/26] Fix Signed-off-by: Kevin Zhang --- .../offline_store/test_offline_push.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/sdk/python/tests/integration/offline_store/test_offline_push.py b/sdk/python/tests/integration/offline_store/test_offline_push.py index 068b7b0a754..44a8053e155 100644 --- a/sdk/python/tests/integration/offline_store/test_offline_push.py +++ b/sdk/python/tests/integration/offline_store/test_offline_push.py @@ -14,6 +14,9 @@ @pytest.mark.universal_online_stores def test_writing_incorrect_order_fails(environment, universal_data_sources): # TODO(kevjumba) handle incorrect order later, for now schema must be in the order that the filesource is in + """This test tests if we have incorrect order when writing to offline store. + Specifically, event_timestamp should be the first column to adhere with the filesource column order. + """ store = environment.feature_store _, _, data_sources = universal_data_sources driver_stats = FeatureView( @@ -43,7 +46,7 @@ def test_writing_incorrect_order_fails(environment, universal_data_sources): assert df["conv_rate"].isnull().all() assert df["avg_daily_trips"].isnull().all() - expected_df = pd.DataFrame.from_dict( + df = pd.DataFrame.from_dict( { "driver_id": [1001, 1002], "event_timestamp": [ts - timedelta(hours=3), ts], @@ -54,7 +57,7 @@ def test_writing_incorrect_order_fails(environment, universal_data_sources): ) with pytest.raises(ValueError): store.write_to_offline_store( - driver_stats.name, expected_df, allow_registry_cache=False + driver_stats.name, df, allow_registry_cache=False ) @@ -62,6 +65,9 @@ def test_writing_incorrect_order_fails(environment, universal_data_sources): @pytest.mark.universal_online_stores def test_writing_incorrect_schema_fails(environment, universal_data_sources): # TODO(kevjumba) handle incorrect order later, for now schema must be in the order that the filesource is in + """This test tests if we have incorrect attribute when writing to offline store. + Specifically, `incorrect_attribute` is an inccorect column to adhere with the filesource column order. + """ store = environment.feature_store _, _, data_sources = universal_data_sources driver_stats = FeatureView( @@ -91,18 +97,18 @@ def test_writing_incorrect_schema_fails(environment, universal_data_sources): assert df["conv_rate"].isnull().all() assert df["avg_daily_trips"].isnull().all() - expected_df = pd.DataFrame.from_dict( + df = pd.DataFrame.from_dict( { "event_timestamp": [ts - timedelta(hours=3), ts], "driver_id": [1001, 1002], "conv_rate": [random.random(), random.random()], - "incorrect_schema": [random.randint(0, 10), random.randint(0, 10)], + "incorrect_attribute": [random.randint(0, 10), random.randint(0, 10)], "created": [ts, ts], }, ) with pytest.raises(ValueError): store.write_to_offline_store( - driver_stats.name, expected_df, allow_registry_cache=False + driver_stats.name, df, allow_registry_cache=False ) @@ -143,6 +149,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour assert df["conv_rate"].isnull().all() assert df["avg_daily_trips"].isnull().all() + # This dataframe has its columns ordered exactly as it is in the parquet file generated by driver_test_data.py. first_df = pd.DataFrame.from_dict( { "event_timestamp": [ts - timedelta(hours=4), ts - timedelta(hours=3)], From b138673af7452d05ab0da9364f96575dfad6ff1f Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Thu, 16 Jun 2022 18:14:19 -0700 Subject: [PATCH 10/26] Fix Signed-off-by: Kevin Zhang --- .../tests/integration/offline_store/test_offline_push.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/sdk/python/tests/integration/offline_store/test_offline_push.py b/sdk/python/tests/integration/offline_store/test_offline_push.py index 44a8053e155..2bdf7751777 100644 --- a/sdk/python/tests/integration/offline_store/test_offline_push.py +++ b/sdk/python/tests/integration/offline_store/test_offline_push.py @@ -56,9 +56,7 @@ def test_writing_incorrect_order_fails(environment, universal_data_sources): }, ) with pytest.raises(ValueError): - store.write_to_offline_store( - driver_stats.name, df, allow_registry_cache=False - ) + store.write_to_offline_store(driver_stats.name, df, allow_registry_cache=False) @pytest.mark.integration @@ -107,9 +105,7 @@ def test_writing_incorrect_schema_fails(environment, universal_data_sources): }, ) with pytest.raises(ValueError): - store.write_to_offline_store( - driver_stats.name, df, allow_registry_cache=False - ) + store.write_to_offline_store(driver_stats.name, df, allow_registry_cache=False) @pytest.mark.integration From f57a129985aa24bdd76f786c81faf45416f5fb2c Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Fri, 17 Jun 2022 09:58:06 -0700 Subject: [PATCH 11/26] Address review comments Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/offline_stores/file.py | 19 +- .../offline_store/test_offline_push.py | 229 ------------------ 2 files changed, 15 insertions(+), 233 deletions(-) delete mode 100644 sdk/python/tests/integration/offline_store/test_offline_push.py diff --git a/sdk/python/feast/infra/offline_stores/file.py b/sdk/python/feast/infra/offline_stores/file.py index b0bf94e3522..194c233f53c 100644 --- a/sdk/python/feast/infra/offline_stores/file.py +++ b/sdk/python/feast/infra/offline_stores/file.py @@ -405,13 +405,24 @@ def write_logged_features( ) @staticmethod - def offline_write_batch(config: RepoConfig, feature_view: FeatureView, data: pyarrow.Table, progress: Optional[Callable[[int], Any]]): + def offline_write_batch( + config: RepoConfig, + feature_view: FeatureView, + data: pyarrow.Table, + progress: Optional[Callable[[int], Any]], + ): if not feature_view.batch_source: - raise ValueError("feature view does not have a batch source to persist offline data") + raise ValueError( + "feature view does not have a batch source to persist offline data" + ) if not isinstance(config.offline_store, FileOfflineStoreConfig): - raise ValueError(f"offline store config is of type {type(config.offline_store)} when file type required") + raise ValueError( + f"offline store config is of type {type(config.offline_store)} when file type required" + ) if not isinstance(feature_view.batch_source, FileSource): - raise ValueError(f"feature view batch source is {type(feature_view.batch_source)} not file source") + raise ValueError( + f"feature view batch source is {type(feature_view.batch_source)} not file source" + ) file_options = feature_view.batch_source.file_options filesystem, path = FileSource.create_filesystem_and_path( file_options.uri, file_options.s3_endpoint_override diff --git a/sdk/python/tests/integration/offline_store/test_offline_push.py b/sdk/python/tests/integration/offline_store/test_offline_push.py deleted file mode 100644 index 2bdf7751777..00000000000 --- a/sdk/python/tests/integration/offline_store/test_offline_push.py +++ /dev/null @@ -1,229 +0,0 @@ -import random -from datetime import datetime, timedelta - -import numpy as np -import pandas as pd -import pytest - -from feast import FeatureView, Field -from feast.types import Float32, Int32 -from tests.integration.feature_repos.universal.entities import driver - - -@pytest.mark.integration -@pytest.mark.universal_online_stores -def test_writing_incorrect_order_fails(environment, universal_data_sources): - # TODO(kevjumba) handle incorrect order later, for now schema must be in the order that the filesource is in - """This test tests if we have incorrect order when writing to offline store. - Specifically, event_timestamp should be the first column to adhere with the filesource column order. - """ - store = environment.feature_store - _, _, data_sources = universal_data_sources - driver_stats = FeatureView( - name="driver_stats", - entities=["driver"], - schema=[ - Field(name="avg_daily_trips", dtype=Int32), - Field(name="conv_rate", dtype=Float32), - ], - source=data_sources.driver, - ) - - now = datetime.utcnow() - ts = pd.Timestamp(now).round("ms") - - entity_df = pd.DataFrame.from_dict( - {"driver_id": [1001, 1002], "event_timestamp": [ts - timedelta(hours=3), ts]} - ) - - store.apply([driver(), driver_stats]) - df = store.get_historical_features( - entity_df=entity_df, - features=["driver_stats:conv_rate", "driver_stats:avg_daily_trips"], - full_feature_names=False, - ).to_df() - - assert df["conv_rate"].isnull().all() - assert df["avg_daily_trips"].isnull().all() - - df = pd.DataFrame.from_dict( - { - "driver_id": [1001, 1002], - "event_timestamp": [ts - timedelta(hours=3), ts], - "conv_rate": [random.random(), random.random()], - "avg_daily_trips": [random.randint(0, 10), random.randint(0, 10)], - "created": [ts, ts], - }, - ) - with pytest.raises(ValueError): - store.write_to_offline_store(driver_stats.name, df, allow_registry_cache=False) - - -@pytest.mark.integration -@pytest.mark.universal_online_stores -def test_writing_incorrect_schema_fails(environment, universal_data_sources): - # TODO(kevjumba) handle incorrect order later, for now schema must be in the order that the filesource is in - """This test tests if we have incorrect attribute when writing to offline store. - Specifically, `incorrect_attribute` is an inccorect column to adhere with the filesource column order. - """ - store = environment.feature_store - _, _, data_sources = universal_data_sources - driver_stats = FeatureView( - name="driver_stats", - entities=["driver"], - schema=[ - Field(name="avg_daily_trips", dtype=Int32), - Field(name="conv_rate", dtype=Float32), - ], - source=data_sources.driver, - ) - - now = datetime.utcnow() - ts = pd.Timestamp(now).round("ms") - - entity_df = pd.DataFrame.from_dict( - {"driver_id": [1001, 1002], "event_timestamp": [ts - timedelta(hours=3), ts]} - ) - - store.apply([driver(), driver_stats]) - df = store.get_historical_features( - entity_df=entity_df, - features=["driver_stats:conv_rate", "driver_stats:avg_daily_trips"], - full_feature_names=False, - ).to_df() - - assert df["conv_rate"].isnull().all() - assert df["avg_daily_trips"].isnull().all() - - df = pd.DataFrame.from_dict( - { - "event_timestamp": [ts - timedelta(hours=3), ts], - "driver_id": [1001, 1002], - "conv_rate": [random.random(), random.random()], - "incorrect_attribute": [random.randint(0, 10), random.randint(0, 10)], - "created": [ts, ts], - }, - ) - with pytest.raises(ValueError): - store.write_to_offline_store(driver_stats.name, df, allow_registry_cache=False) - - -@pytest.mark.integration -@pytest.mark.universal_online_stores -def test_writing_consecutively_to_offline_store(environment, universal_data_sources): - store = environment.feature_store - _, _, data_sources = universal_data_sources - driver_stats = FeatureView( - name="driver_stats", - entities=["driver"], - schema=[ - Field(name="avg_daily_trips", dtype=Int32), - Field(name="conv_rate", dtype=Float32), - Field(name="acc_rate", dtype=Float32), - ], - source=data_sources.driver, - ttl=timedelta(minutes=10), - ) - - now = datetime.utcnow() - ts = pd.Timestamp(now, unit="ns") - - entity_df = pd.DataFrame.from_dict( - { - "driver_id": [1001, 1001], - "event_timestamp": [ts - timedelta(hours=4), ts - timedelta(hours=3)], - } - ) - - store.apply([driver(), driver_stats]) - df = store.get_historical_features( - entity_df=entity_df, - features=["driver_stats:conv_rate", "driver_stats:avg_daily_trips"], - full_feature_names=False, - ).to_df() - - assert df["conv_rate"].isnull().all() - assert df["avg_daily_trips"].isnull().all() - - # This dataframe has its columns ordered exactly as it is in the parquet file generated by driver_test_data.py. - first_df = pd.DataFrame.from_dict( - { - "event_timestamp": [ts - timedelta(hours=4), ts - timedelta(hours=3)], - "driver_id": [1001, 1001], - "conv_rate": [random.random(), random.random()], - "acc_rate": [random.random(), random.random()], - "avg_daily_trips": [random.randint(0, 10), random.randint(0, 10)], - "created": [ts, ts], - }, - ) - store.write_to_offline_store( - driver_stats.name, first_df, allow_registry_cache=False - ) - - after_write_df = store.get_historical_features( - entity_df=entity_df, - features=["driver_stats:conv_rate", "driver_stats:avg_daily_trips"], - full_feature_names=False, - ).to_df() - - assert len(after_write_df) == len(first_df) - assert np.where( - after_write_df["conv_rate"].reset_index(drop=True) - == first_df["conv_rate"].reset_index(drop=True) - ) - assert np.where( - after_write_df["avg_daily_trips"].reset_index(drop=True) - == first_df["avg_daily_trips"].reset_index(drop=True) - ) - - second_df = pd.DataFrame.from_dict( - { - "event_timestamp": [ts - timedelta(hours=1), ts], - "driver_id": [1001, 1001], - "conv_rate": [random.random(), random.random()], - "acc_rate": [random.random(), random.random()], - "avg_daily_trips": [random.randint(0, 10), random.randint(0, 10)], - "created": [ts, ts], - }, - ) - - store.write_to_offline_store( - driver_stats.name, second_df, allow_registry_cache=False - ) - - entity_df = pd.DataFrame.from_dict( - { - "driver_id": [1001, 1001, 1001, 1001], - "event_timestamp": [ - ts - timedelta(hours=4), - ts - timedelta(hours=3), - ts - timedelta(hours=1), - ts, - ], - } - ) - - after_write_df = store.get_historical_features( - entity_df=entity_df, - features=[ - "driver_stats:conv_rate", - "driver_stats:acc_rate", - "driver_stats:avg_daily_trips", - ], - full_feature_names=False, - ).to_df() - - expected_df = pd.concat([first_df, second_df]) - assert len(after_write_df) == len(expected_df) - assert np.where( - after_write_df["conv_rate"].reset_index(drop=True) - == expected_df["conv_rate"].reset_index(drop=True) - ) - assert np.where( - after_write_df["acc_rate"].reset_index(drop=True) - == expected_df["acc_rate"].reset_index(drop=True) - ) - assert np.where( - after_write_df["avg_daily_trips"].reset_index(drop=True) - == expected_df["avg_daily_trips"].reset_index(drop=True) - ) From 1800276467dd2ffb59ac3f5ede4b511ad11d9669 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Fri, 17 Jun 2022 10:31:43 -0700 Subject: [PATCH 12/26] Add redshift function Signed-off-by: Kevin Zhang --- .../feast/infra/offline_stores/redshift.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/sdk/python/feast/infra/offline_stores/redshift.py b/sdk/python/feast/infra/offline_stores/redshift.py index a5483e8140e..8d908898f9b 100644 --- a/sdk/python/feast/infra/offline_stores/redshift.py +++ b/sdk/python/feast/infra/offline_stores/redshift.py @@ -11,6 +11,7 @@ Optional, Tuple, Union, + Any, ) import numpy as np @@ -297,6 +298,34 @@ def write_logged_features( fail_if_exists=False, ) + @staticmethod + def offline_write_batch(config: RepoConfig, feature_view: FeatureView, table: pyarrow.Table, progress: Optional[Callable[[int], Any]]): + if not feature_view.batch_source: + raise ValueError("feature view does not have a batch source to persist offline data") + if not isinstance(config.offline_store, RedshiftOfflineStoreConfig): + raise ValueError(f"offline store config is of type {type(config.offline_store)} when file type required") + if not isinstance(feature_view.batch_source, RedshiftSource): + raise ValueError(f"feature view batch source is {type(feature_view.batch_source)} not file source") + redshift_options = feature_view.batch_source.redshift_options + redshift_client = aws_utils.get_redshift_data_client( + config.offline_store.region + ) + s3_resource = aws_utils.get_s3_resource(config.offline_store.region) + + table.reset_index(drop=True, inplace=True) + + aws_utils.upload_arrow_table_to_redshift( + table=table, + redshift_data_client=redshift_client, + cluster_id=config.offline_store.cluster_id, + database=redshift_options.database, + user=config.offline_store.user, + s3_resource=s3_resource, + s3_path=f"{config.offline_store.s3_staging_location}/push/{uuid.uuid4()}.parquet", + iam_role=config.offline_store.iam_role, + table_name=redshift_options.table, + fail_if_exists=False, + ) class RedshiftRetrievalJob(RetrievalJob): def __init__( From b277eb1fecebabdc201d4ea7e23e2889140bea3f Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Mon, 20 Jun 2022 17:10:02 -0700 Subject: [PATCH 13/26] Add redshift Signed-off-by: Kevin Zhang --- sdk/python/feast/feature_store.py | 2 + .../feast/infra/offline_stores/redshift.py | 20 +++++- sdk/python/feast/infra/utils/aws_utils.py | 65 ++++++++++++++++++- sdk/python/tests/conftest.py | 15 ++++- .../feature_repos/repo_configuration.py | 8 +++ .../offline_store/test_offline_write.py | 26 +++++--- 6 files changed, 120 insertions(+), 16 deletions(-) diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index 25fb037f87f..9c2ea8a2762 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -1423,6 +1423,8 @@ def _write_to_offline_store( feature_view = self.get_feature_view( feature_view_name, allow_registry_cache=allow_registry_cache ) + df.reset_index(drop=True) + table = pa.Table.from_pandas(df) provider = self._get_provider() provider.ingest_df_to_offline_store(feature_view, table) diff --git a/sdk/python/feast/infra/offline_stores/redshift.py b/sdk/python/feast/infra/offline_stores/redshift.py index 8d908898f9b..19d13d8298a 100644 --- a/sdk/python/feast/infra/offline_stores/redshift.py +++ b/sdk/python/feast/infra/offline_stores/redshift.py @@ -13,6 +13,7 @@ Union, Any, ) +from feast.type_map import redshift_to_feast_value_type, feast_value_type_to_pa import numpy as np import pandas as pd @@ -310,9 +311,23 @@ def offline_write_batch(config: RepoConfig, feature_view: FeatureView, table: py redshift_client = aws_utils.get_redshift_data_client( config.offline_store.region ) - s3_resource = aws_utils.get_s3_resource(config.offline_store.region) - table.reset_index(drop=True, inplace=True) + column_name_to_type = feature_view.batch_source.get_table_column_names_and_types(config) + pa_schema_list = [] + column_names = [] + for column_name, redshift_type in column_name_to_type: + pa_schema_list.append((column_name, feast_value_type_to_pa(redshift_to_feast_value_type(redshift_type)))) + column_names.append(column_name) + pa_schema = pa.schema(pa_schema_list) + if column_names != table.column_names: + raise ValueError( + f"Input dataframe has incorrect schema or wrong order, expected columns are: {column_names}" + ) + + if table.schema != pa_schema: + table = table.cast(pa_schema) + + s3_resource = aws_utils.get_s3_resource(config.offline_store.region) aws_utils.upload_arrow_table_to_redshift( table=table, @@ -324,6 +339,7 @@ def offline_write_batch(config: RepoConfig, feature_view: FeatureView, table: py s3_path=f"{config.offline_store.s3_staging_location}/push/{uuid.uuid4()}.parquet", iam_role=config.offline_store.iam_role, table_name=redshift_options.table, + schema=pa_schema, fail_if_exists=False, ) diff --git a/sdk/python/feast/infra/utils/aws_utils.py b/sdk/python/feast/infra/utils/aws_utils.py index bb75160a873..ef1370cf5f4 100644 --- a/sdk/python/feast/infra/utils/aws_utils.py +++ b/sdk/python/feast/infra/utils/aws_utils.py @@ -234,6 +234,23 @@ def upload_df_to_redshift( table_name=table_name, ) +def delete_redshift_table( + redshift_data_client, + cluster_id: str, + database: str, + user: str, + table_name: str, +): + drop_query = ( + f"DROP {table_name} IF EXISTS" + ) + execute_redshift_statement( + redshift_data_client, + cluster_id, + database, + user, + drop_query, + ) def upload_arrow_table_to_redshift( table: Union[pyarrow.Table, Path], @@ -320,7 +337,7 @@ def upload_arrow_table_to_redshift( cluster_id, database, user, - f"{create_query}; {copy_query}", + f"{create_query}; {copy_query};", ) finally: # Clean up S3 temporary data @@ -370,6 +387,52 @@ def temporarily_upload_df_to_redshift( redshift_data_client, cluster_id, database, user, f"DROP TABLE {table_name}", ) +@contextlib.contextmanager +def temporarily_upload_arrow_table_to_redshift( + table: Union[pyarrow.Table, Path], + redshift_data_client, + cluster_id: str, + database: str, + user: str, + s3_resource, + iam_role: str, + s3_path: str, + table_name: str, + schema: Optional[pyarrow.Schema] = None, + fail_if_exists: bool = True, +) -> Iterator[None]: + """Uploads a Arrow Table to Redshift as a new table with cleanup logic. + + This is essentially the same as upload_arrow_table_to_redshift (check out its docstring for full details), + but unlike it this method is a generator and should be used with `with` block. For example: + + >>> with temporarily_upload_arrow_table_to_redshift(...): # doctest: +SKIP + >>> # Use `table_name` table in Redshift here + >>> # `table_name` will not exist at this point, since it's cleaned up by the `with` block + + """ + # Upload the dataframe to Redshift + upload_arrow_table_to_redshift( + table, + redshift_data_client, + cluster_id, + database, + user, + s3_resource, + s3_path, + iam_role, + table_name, + schema, + fail_if_exists, + ) + + yield + + # Clean up the uploaded Redshift table + execute_redshift_statement( + redshift_data_client, cluster_id, database, user, f"DROP TABLE {table_name}", + ) + def download_s3_directory(s3_resource, bucket: str, key: str, local_dir: str): """Download the S3 directory to a local disk""" diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 671acb3b92a..b18ab7e7109 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -31,6 +31,7 @@ IntegrationTestRepoConfig, ) from tests.integration.feature_repos.repo_configuration import ( + OFFLINE_STORE_TO_PROVIDER_CONFIG, AVAILABLE_OFFLINE_STORES, AVAILABLE_ONLINE_STORES, Environment, @@ -196,16 +197,24 @@ def pytest_generate_tests(metafunc: pytest.Metafunc): """ if "environment" in metafunc.fixturenames: markers = {m.name: m for m in metafunc.definition.own_markers} - + offline_stores = None if "universal_offline_stores" in markers: - offline_stores = AVAILABLE_OFFLINE_STORES + # Offline stores can be explicitly requested + if "only" in markers["universal_offline_stores"].kwargs: + offline_stores = [ + OFFLINE_STORE_TO_PROVIDER_CONFIG.get(store_name) + for store_name in markers["universal_offline_stores"].kwargs["only"] + if store_name in OFFLINE_STORE_TO_PROVIDER_CONFIG + ] + else: + offline_stores = AVAILABLE_OFFLINE_STORES else: # default offline store for testing online store dimension offline_stores = [("local", FileDataSourceCreator)] online_stores = None if "universal_online_stores" in markers: - # Online stores are explicitly requested + # Online stores can be explicitly requested if "only" in markers["universal_online_stores"].kwargs: online_stores = [ AVAILABLE_ONLINE_STORES.get(store_name) diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 5a48115dbed..fd49eb79e7f 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -74,6 +74,14 @@ "connection_string": "127.0.0.1:6001,127.0.0.1:6002,127.0.0.1:6003", } +OFFLINE_STORE_TO_PROVIDER_CONFIG : Dict[ + str, DataSourceCreator] = { + "file": ("local", FileDataSourceCreator), + "gcp": ("gcp", BigQueryDataSourceCreator), + "redshift": ("aws", RedshiftDataSourceCreator), + "snowflake": ("aws", RedshiftDataSourceCreator), +} + AVAILABLE_OFFLINE_STORES: List[Tuple[str, Type[DataSourceCreator]]] = [ ("local", FileDataSourceCreator), ] diff --git a/sdk/python/tests/integration/offline_store/test_offline_write.py b/sdk/python/tests/integration/offline_store/test_offline_write.py index 41f6ea89fa2..9557e987148 100644 --- a/sdk/python/tests/integration/offline_store/test_offline_write.py +++ b/sdk/python/tests/integration/offline_store/test_offline_write.py @@ -9,10 +9,10 @@ from feast.types import Float32, Int32 from tests.integration.feature_repos.universal.entities import driver - @pytest.mark.integration -@pytest.mark.universal_online_stores -def test_writing_incorrect_order_fails(environment, universal_data_sources): +@pytest.mark.universal_offline_stores(only=["file", "redshift"]) +@pytest.mark.universal_online_stores(only=["sqlite"]) +def test_writing_columns_in_incorrect_order_fails(environment, universal_data_sources): # TODO(kevjumba) handle incorrect order later, for now schema must be in the order that the filesource is in store = environment.feature_store _, _, data_sources = universal_data_sources @@ -59,7 +59,8 @@ def test_writing_incorrect_order_fails(environment, universal_data_sources): @pytest.mark.integration -@pytest.mark.universal_online_stores +@pytest.mark.universal_offline_stores(only=["file", "redshift"]) +@pytest.mark.universal_online_stores(only=["sqlite"]) def test_writing_incorrect_schema_fails(environment, universal_data_sources): # TODO(kevjumba) handle incorrect order later, for now schema must be in the order that the filesource is in store = environment.feature_store @@ -105,9 +106,9 @@ def test_writing_incorrect_schema_fails(environment, universal_data_sources): driver_stats.name, expected_df, allow_registry_cache=False ) - @pytest.mark.integration -@pytest.mark.universal_online_stores +@pytest.mark.universal_offline_stores(only=["file", "redshift"]) +@pytest.mark.universal_online_stores(only=["sqlite"]) def test_writing_consecutively_to_offline_store(environment, universal_data_sources): store = environment.feature_store _, _, data_sources = universal_data_sources @@ -124,7 +125,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour ) now = datetime.utcnow() - ts = pd.Timestamp(now, unit="ns") + ts = pd.Timestamp(now, unit="ms", tz="UTC").round("ms") entity_df = pd.DataFrame.from_dict( { @@ -145,7 +146,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour first_df = pd.DataFrame.from_dict( { - "event_timestamp": [ts - timedelta(hours=4), ts - timedelta(hours=3)], + "event_timestamp": [now-timedelta(hours=4), now - timedelta(hours=3)], "driver_id": [1001, 1001], "conv_rate": [random.random(), random.random()], "acc_rate": [random.random(), random.random()], @@ -153,13 +154,18 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour "created": [ts, ts], }, ) + store._write_to_offline_store( driver_stats.name, first_df, allow_registry_cache=False ) after_write_df = store.get_historical_features( entity_df=entity_df, - features=["driver_stats:conv_rate", "driver_stats:avg_daily_trips"], + features=[ + "driver_stats:conv_rate", + "driver_stats:acc_rate", + "driver_stats:avg_daily_trips", + ], full_feature_names=False, ).to_df() @@ -223,4 +229,4 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour assert np.where( after_write_df["avg_daily_trips"].reset_index(drop=True) == expected_df["avg_daily_trips"].reset_index(drop=True) - ) + ) \ No newline at end of file From 646cf3da442677124c97605124b0f1cc3aed17dd Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Mon, 20 Jun 2022 17:22:03 -0700 Subject: [PATCH 14/26] Fix Signed-off-by: Kevin Zhang --- .../integration/offline_store/test_offline_write.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/sdk/python/tests/integration/offline_store/test_offline_write.py b/sdk/python/tests/integration/offline_store/test_offline_write.py index 9557e987148..f1775db6bf1 100644 --- a/sdk/python/tests/integration/offline_store/test_offline_write.py +++ b/sdk/python/tests/integration/offline_store/test_offline_write.py @@ -125,7 +125,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour ) now = datetime.utcnow() - ts = pd.Timestamp(now, unit="ms", tz="UTC").round("ms") + ts = pd.Timestamp(now, unit="ns") entity_df = pd.DataFrame.from_dict( { @@ -146,7 +146,7 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour first_df = pd.DataFrame.from_dict( { - "event_timestamp": [now-timedelta(hours=4), now - timedelta(hours=3)], + "event_timestamp": [ts - timedelta(hours=4), ts - timedelta(hours=3)], "driver_id": [1001, 1001], "conv_rate": [random.random(), random.random()], "acc_rate": [random.random(), random.random()], @@ -154,18 +154,13 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour "created": [ts, ts], }, ) - store._write_to_offline_store( driver_stats.name, first_df, allow_registry_cache=False ) after_write_df = store.get_historical_features( entity_df=entity_df, - features=[ - "driver_stats:conv_rate", - "driver_stats:acc_rate", - "driver_stats:avg_daily_trips", - ], + features=["driver_stats:conv_rate", "driver_stats:avg_daily_trips"], full_feature_names=False, ).to_df() From 1d72fd1aeb130cd718ed00a83ecf2b6d46256e91 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Mon, 20 Jun 2022 17:23:45 -0700 Subject: [PATCH 15/26] Lint Signed-off-by: Kevin Zhang --- .../feast/infra/offline_stores/redshift.py | 35 +++++++--- sdk/python/feast/infra/utils/aws_utils.py | 19 ++--- sdk/python/tests/conftest.py | 69 ++++++++++--------- .../feature_repos/repo_configuration.py | 11 ++- .../offline_store/test_offline_write.py | 4 +- 5 files changed, 76 insertions(+), 62 deletions(-) diff --git a/sdk/python/feast/infra/offline_stores/redshift.py b/sdk/python/feast/infra/offline_stores/redshift.py index 19d13d8298a..70baf62c096 100644 --- a/sdk/python/feast/infra/offline_stores/redshift.py +++ b/sdk/python/feast/infra/offline_stores/redshift.py @@ -3,6 +3,7 @@ from datetime import datetime from pathlib import Path from typing import ( + Any, Callable, ContextManager, Dict, @@ -11,9 +12,7 @@ Optional, Tuple, Union, - Any, ) -from feast.type_map import redshift_to_feast_value_type, feast_value_type_to_pa import numpy as np import pandas as pd @@ -43,6 +42,7 @@ from feast.registry import BaseRegistry from feast.repo_config import FeastConfigBaseModel, RepoConfig from feast.saved_dataset import SavedDatasetStorage +from feast.type_map import feast_value_type_to_pa, redshift_to_feast_value_type from feast.usage import log_exceptions_and_usage @@ -300,23 +300,41 @@ def write_logged_features( ) @staticmethod - def offline_write_batch(config: RepoConfig, feature_view: FeatureView, table: pyarrow.Table, progress: Optional[Callable[[int], Any]]): + def offline_write_batch( + config: RepoConfig, + feature_view: FeatureView, + table: pyarrow.Table, + progress: Optional[Callable[[int], Any]], + ): if not feature_view.batch_source: - raise ValueError("feature view does not have a batch source to persist offline data") + raise ValueError( + "feature view does not have a batch source to persist offline data" + ) if not isinstance(config.offline_store, RedshiftOfflineStoreConfig): - raise ValueError(f"offline store config is of type {type(config.offline_store)} when file type required") + raise ValueError( + f"offline store config is of type {type(config.offline_store)} when file type required" + ) if not isinstance(feature_view.batch_source, RedshiftSource): - raise ValueError(f"feature view batch source is {type(feature_view.batch_source)} not file source") + raise ValueError( + f"feature view batch source is {type(feature_view.batch_source)} not file source" + ) redshift_options = feature_view.batch_source.redshift_options redshift_client = aws_utils.get_redshift_data_client( config.offline_store.region ) - column_name_to_type = feature_view.batch_source.get_table_column_names_and_types(config) + column_name_to_type = feature_view.batch_source.get_table_column_names_and_types( + config + ) pa_schema_list = [] column_names = [] for column_name, redshift_type in column_name_to_type: - pa_schema_list.append((column_name, feast_value_type_to_pa(redshift_to_feast_value_type(redshift_type)))) + pa_schema_list.append( + ( + column_name, + feast_value_type_to_pa(redshift_to_feast_value_type(redshift_type)), + ) + ) column_names.append(column_name) pa_schema = pa.schema(pa_schema_list) if column_names != table.column_names: @@ -343,6 +361,7 @@ def offline_write_batch(config: RepoConfig, feature_view: FeatureView, table: py fail_if_exists=False, ) + class RedshiftRetrievalJob(RetrievalJob): def __init__( self, diff --git a/sdk/python/feast/infra/utils/aws_utils.py b/sdk/python/feast/infra/utils/aws_utils.py index ef1370cf5f4..7badda98460 100644 --- a/sdk/python/feast/infra/utils/aws_utils.py +++ b/sdk/python/feast/infra/utils/aws_utils.py @@ -234,24 +234,16 @@ def upload_df_to_redshift( table_name=table_name, ) + def delete_redshift_table( - redshift_data_client, - cluster_id: str, - database: str, - user: str, - table_name: str, + redshift_data_client, cluster_id: str, database: str, user: str, table_name: str, ): - drop_query = ( - f"DROP {table_name} IF EXISTS" - ) + drop_query = f"DROP {table_name} IF EXISTS" execute_redshift_statement( - redshift_data_client, - cluster_id, - database, - user, - drop_query, + redshift_data_client, cluster_id, database, user, drop_query, ) + def upload_arrow_table_to_redshift( table: Union[pyarrow.Table, Path], redshift_data_client, @@ -387,6 +379,7 @@ def temporarily_upload_df_to_redshift( redshift_data_client, cluster_id, database, user, f"DROP TABLE {table_name}", ) + @contextlib.contextmanager def temporarily_upload_arrow_table_to_redshift( table: Union[pyarrow.Table, Path], diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index b18ab7e7109..c340b7a289a 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -31,9 +31,9 @@ IntegrationTestRepoConfig, ) from tests.integration.feature_repos.repo_configuration import ( - OFFLINE_STORE_TO_PROVIDER_CONFIG, AVAILABLE_OFFLINE_STORES, AVAILABLE_ONLINE_STORES, + OFFLINE_STORE_TO_PROVIDER_CONFIG, Environment, TestData, construct_test_environment, @@ -249,40 +249,41 @@ def pytest_generate_tests(metafunc: pytest.Metafunc): extra_dimensions.append({"go_feature_retrieval": True}) configs = [] - for provider, offline_store_creator in offline_stores: - for online_store, online_store_creator in online_stores: - for dim in extra_dimensions: - config = { - "provider": provider, - "offline_store_creator": offline_store_creator, - "online_store": online_store, - "online_store_creator": online_store_creator, - **dim, - } - # temporary Go works only with redis - if config.get("go_feature_retrieval") and ( - not isinstance(online_store, dict) - or online_store["type"] != "redis" - ): - continue - - # aws lambda works only with dynamo - if ( - config.get("python_feature_server") - and config.get("provider") == "aws" - and ( + if offline_stores: + for provider, offline_store_creator in offline_stores: + for online_store, online_store_creator in online_stores: + for dim in extra_dimensions: + config = { + "provider": provider, + "offline_store_creator": offline_store_creator, + "online_store": online_store, + "online_store_creator": online_store_creator, + **dim, + } + # temporary Go works only with redis + if config.get("go_feature_retrieval") and ( not isinstance(online_store, dict) - or online_store["type"] != "dynamodb" - ) - ): - continue - - c = IntegrationTestRepoConfig(**config) - - if c not in _config_cache: - _config_cache[c] = c - - configs.append(_config_cache[c]) + or online_store["type"] != "redis" + ): + continue + + # aws lambda works only with dynamo + if ( + config.get("python_feature_server") + and config.get("provider") == "aws" + and ( + not isinstance(online_store, dict) + or online_store["type"] != "dynamodb" + ) + ): + continue + + c = IntegrationTestRepoConfig(**config) + + if c not in _config_cache: + _config_cache[c] = c + + configs.append(_config_cache[c]) metafunc.parametrize( "environment", configs, indirect=True, ids=[str(c) for c in configs] diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index fd49eb79e7f..f4d5defcad8 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -74,12 +74,11 @@ "connection_string": "127.0.0.1:6001,127.0.0.1:6002,127.0.0.1:6003", } -OFFLINE_STORE_TO_PROVIDER_CONFIG : Dict[ - str, DataSourceCreator] = { - "file": ("local", FileDataSourceCreator), - "gcp": ("gcp", BigQueryDataSourceCreator), - "redshift": ("aws", RedshiftDataSourceCreator), - "snowflake": ("aws", RedshiftDataSourceCreator), +OFFLINE_STORE_TO_PROVIDER_CONFIG: Dict[str, DataSourceCreator] = { + "file": ("local", FileDataSourceCreator), + "gcp": ("gcp", BigQueryDataSourceCreator), + "redshift": ("aws", RedshiftDataSourceCreator), + "snowflake": ("aws", RedshiftDataSourceCreator), } AVAILABLE_OFFLINE_STORES: List[Tuple[str, Type[DataSourceCreator]]] = [ diff --git a/sdk/python/tests/integration/offline_store/test_offline_write.py b/sdk/python/tests/integration/offline_store/test_offline_write.py index f1775db6bf1..5e7a242513e 100644 --- a/sdk/python/tests/integration/offline_store/test_offline_write.py +++ b/sdk/python/tests/integration/offline_store/test_offline_write.py @@ -9,6 +9,7 @@ from feast.types import Float32, Int32 from tests.integration.feature_repos.universal.entities import driver + @pytest.mark.integration @pytest.mark.universal_offline_stores(only=["file", "redshift"]) @pytest.mark.universal_online_stores(only=["sqlite"]) @@ -106,6 +107,7 @@ def test_writing_incorrect_schema_fails(environment, universal_data_sources): driver_stats.name, expected_df, allow_registry_cache=False ) + @pytest.mark.integration @pytest.mark.universal_offline_stores(only=["file", "redshift"]) @pytest.mark.universal_online_stores(only=["sqlite"]) @@ -224,4 +226,4 @@ def test_writing_consecutively_to_offline_store(environment, universal_data_sour assert np.where( after_write_df["avg_daily_trips"].reset_index(drop=True) == expected_df["avg_daily_trips"].reset_index(drop=True) - ) \ No newline at end of file + ) From 89a0ff0f758834a1b823dd5d4d6923fdb7fdaf66 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 21 Jun 2022 08:31:47 -0700 Subject: [PATCH 16/26] fix Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/offline_stores/redshift.py | 4 ++-- sdk/python/tests/conftest.py | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/infra/offline_stores/redshift.py b/sdk/python/feast/infra/offline_stores/redshift.py index 70baf62c096..badbd0b5a9e 100644 --- a/sdk/python/feast/infra/offline_stores/redshift.py +++ b/sdk/python/feast/infra/offline_stores/redshift.py @@ -351,12 +351,12 @@ def offline_write_batch( table=table, redshift_data_client=redshift_client, cluster_id=config.offline_store.cluster_id, - database=redshift_options.database, + database=redshift_options.database or config.offline_store.database, # Users can define database in the source if needed but it's not required. user=config.offline_store.user, s3_resource=s3_resource, s3_path=f"{config.offline_store.s3_staging_location}/push/{uuid.uuid4()}.parquet", iam_role=config.offline_store.iam_role, - table_name=redshift_options.table, + table_name=redshift_options.table , schema=pa_schema, fail_if_exists=False, ) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index c340b7a289a..6c69cfa9103 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -284,6 +284,9 @@ def pytest_generate_tests(metafunc: pytest.Metafunc): _config_cache[c] = c configs.append(_config_cache[c]) + else: + # No offline stores requested -> setting the default or first available + offline_stores = ("local", FileDataSourceCreator) metafunc.parametrize( "environment", configs, indirect=True, ids=[str(c) for c in configs] From 14919d8981ae1fc17b4e855c9827653fd2e3928a Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 21 Jun 2022 08:33:00 -0700 Subject: [PATCH 17/26] fix Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/offline_stores/redshift.py | 5 +++-- sdk/python/tests/conftest.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/sdk/python/feast/infra/offline_stores/redshift.py b/sdk/python/feast/infra/offline_stores/redshift.py index badbd0b5a9e..27e439ef7de 100644 --- a/sdk/python/feast/infra/offline_stores/redshift.py +++ b/sdk/python/feast/infra/offline_stores/redshift.py @@ -351,12 +351,13 @@ def offline_write_batch( table=table, redshift_data_client=redshift_client, cluster_id=config.offline_store.cluster_id, - database=redshift_options.database or config.offline_store.database, # Users can define database in the source if needed but it's not required. + database=redshift_options.database + or config.offline_store.database, # Users can define database in the source if needed but it's not required. user=config.offline_store.user, s3_resource=s3_resource, s3_path=f"{config.offline_store.s3_staging_location}/push/{uuid.uuid4()}.parquet", iam_role=config.offline_store.iam_role, - table_name=redshift_options.table , + table_name=redshift_options.table, schema=pa_schema, fail_if_exists=False, ) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 6c69cfa9103..bf69a85fa31 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -286,7 +286,7 @@ def pytest_generate_tests(metafunc: pytest.Metafunc): configs.append(_config_cache[c]) else: # No offline stores requested -> setting the default or first available - offline_stores = ("local", FileDataSourceCreator) + offline_stores = [("local", FileDataSourceCreator)] metafunc.parametrize( "environment", configs, indirect=True, ids=[str(c) for c in configs] From 01f80d6895bb04eea2fa4ed5364f7284c94bea0a Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 21 Jun 2022 08:34:52 -0700 Subject: [PATCH 18/26] Fix errors Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/offline_stores/redshift.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/infra/offline_stores/redshift.py b/sdk/python/feast/infra/offline_stores/redshift.py index 27e439ef7de..943bac502cb 100644 --- a/sdk/python/feast/infra/offline_stores/redshift.py +++ b/sdk/python/feast/infra/offline_stores/redshift.py @@ -312,11 +312,11 @@ def offline_write_batch( ) if not isinstance(config.offline_store, RedshiftOfflineStoreConfig): raise ValueError( - f"offline store config is of type {type(config.offline_store)} when file type required" + f"offline store config is of type {type(config.offline_store)} when redshift type required" ) if not isinstance(feature_view.batch_source, RedshiftSource): raise ValueError( - f"feature view batch source is {type(feature_view.batch_source)} not file source" + f"feature view batch source is {type(feature_view.batch_source)} not redshift source" ) redshift_options = feature_view.batch_source.redshift_options redshift_client = aws_utils.get_redshift_data_client( From c52b476fdcacdcf54fc637daa96acb135a4e6d38 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 21 Jun 2022 14:45:47 -0700 Subject: [PATCH 19/26] Fix test Signed-off-by: Kevin Zhang --- .../online_store/test_universal_online.py | 1202 ++++++++--------- 1 file changed, 601 insertions(+), 601 deletions(-) diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index c068e041116..3d066e7ba70 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -441,604 +441,604 @@ def test_online_retrieval_with_event_timestamps( ) -@pytest.mark.integration -@pytest.mark.universal_online_stores -# @pytest.mark.goserver Disabling because the go fs tests are flaking in CI. TODO(achals): uncomment after fixed. -@pytest.mark.parametrize("full_feature_names", [True, False], ids=lambda v: str(v)) -def test_stream_feature_view_online_retrieval( - environment, universal_data_sources, feature_server_endpoint, full_feature_names -): - """ - Tests materialization and online retrieval for stream feature views. - - This test is separate from test_online_retrieval since combining feature views and - stream feature views into a single test resulted in test flakiness. This is tech - debt that should be resolved soon. - """ - # Set up feature store. - fs = environment.feature_store - entities, datasets, data_sources = universal_data_sources - feature_views = construct_universal_feature_views(data_sources) - pushable_feature_view = feature_views.pushed_locations - fs.apply([location(), pushable_feature_view]) - - # Materialize. - fs.materialize( - environment.start_date - timedelta(days=1), - environment.end_date + timedelta(days=1), - ) - - # Get online features by randomly sampling 10 entities that exist in the batch source. - sample_locations = datasets.location_df.sample(10)["location_id"] - entity_rows = [ - {"location_id": sample_location} for sample_location in sample_locations - ] - - feature_refs = [ - "pushable_location_stats:temperature", - ] - unprefixed_feature_refs = [f.rsplit(":", 1)[-1] for f in feature_refs if ":" in f] - - online_features_dict = get_online_features_dict( - environment=environment, - endpoint=feature_server_endpoint, - features=feature_refs, - entity_rows=entity_rows, - full_feature_names=full_feature_names, - ) - - # Check that the response has the expected set of keys. - keys = set(online_features_dict.keys()) - expected_keys = set( - f.replace(":", "__") if full_feature_names else f.split(":")[-1] - for f in feature_refs - ) | {"location_id"} - assert ( - keys == expected_keys - ), f"Response keys are different from expected: {keys - expected_keys} (extra) and {expected_keys - keys} (missing)" - - # Check that the feature values match. - tc = unittest.TestCase() - for i, entity_row in enumerate(entity_rows): - df_features = get_latest_feature_values_from_location_df( - entity_row, datasets.location_df - ) - - assert df_features["location_id"] == online_features_dict["location_id"][i] - for unprefixed_feature_ref in unprefixed_feature_refs: - tc.assertAlmostEqual( - df_features[unprefixed_feature_ref], - online_features_dict[ - response_feature_name( - unprefixed_feature_ref, feature_refs, full_feature_names - ) - ][i], - delta=0.0001, - ) - - -@pytest.mark.integration -@pytest.mark.universal_online_stores -# @pytest.mark.goserver Disabling because the go fs tests are flaking in CI. TODO(achals): uncomment after fixed. -@pytest.mark.parametrize("full_feature_names", [True, False], ids=lambda v: str(v)) -def test_online_retrieval( - environment, universal_data_sources, feature_server_endpoint, full_feature_names -): - fs = environment.feature_store - entities, datasets, data_sources = universal_data_sources - feature_views = construct_universal_feature_views(data_sources) - - feature_service = FeatureService( - "convrate_plus100", - features=[ - feature_views.driver[["conv_rate"]], - feature_views.driver_odfv, - feature_views.customer[["current_balance"]], - ], - ) - feature_service_entity_mapping = FeatureService( - name="entity_mapping", - features=[ - feature_views.location.with_name("origin").with_join_key_map( - {"location_id": "origin_id"} - ), - feature_views.location.with_name("destination").with_join_key_map( - {"location_id": "destination_id"} - ), - ], - ) - - feast_objects = [] - feast_objects.extend(feature_views.values()) - feast_objects.extend( - [ - driver(), - customer(), - location(), - feature_service, - feature_service_entity_mapping, - ] - ) - fs.apply(feast_objects) - fs.materialize( - environment.start_date - timedelta(days=1), - environment.end_date + timedelta(days=1), - ) - - entity_sample = datasets.orders_df.sample(10)[ - ["customer_id", "driver_id", "order_id", "event_timestamp"] - ] - orders_df = datasets.orders_df[ - ( - datasets.orders_df["customer_id"].isin(entity_sample["customer_id"]) - & datasets.orders_df["driver_id"].isin(entity_sample["driver_id"]) - ) - ] - - sample_drivers = entity_sample["driver_id"] - drivers_df = datasets.driver_df[ - datasets.driver_df["driver_id"].isin(sample_drivers) - ] - - sample_customers = entity_sample["customer_id"] - customers_df = datasets.customer_df[ - datasets.customer_df["customer_id"].isin(sample_customers) - ] - - location_pairs = np.array(list(itertools.permutations(entities.location_vals, 2))) - sample_location_pairs = location_pairs[ - np.random.choice(len(location_pairs), 10) - ].T.tolist() - origins_df = datasets.location_df[ - datasets.location_df["location_id"].isin(sample_location_pairs[0]) - ] - destinations_df = datasets.location_df[ - datasets.location_df["location_id"].isin(sample_location_pairs[1]) - ] - - global_df = datasets.global_df - - entity_rows = [ - {"driver_id": d, "customer_id": c, "val_to_add": 50} - for (d, c) in zip(sample_drivers, sample_customers) - ] - - feature_refs = [ - "driver_stats:conv_rate", - "driver_stats:avg_daily_trips", - "customer_profile:current_balance", - "customer_profile:avg_passenger_count", - "customer_profile:lifetime_trip_count", - "conv_rate_plus_100:conv_rate_plus_100", - "conv_rate_plus_100:conv_rate_plus_val_to_add", - "order:order_is_success", - "global_stats:num_rides", - "global_stats:avg_ride_length", - ] - unprefixed_feature_refs = [f.rsplit(":", 1)[-1] for f in feature_refs if ":" in f] - # Remove the on demand feature view output features, since they're not present in the source dataframe - unprefixed_feature_refs.remove("conv_rate_plus_100") - unprefixed_feature_refs.remove("conv_rate_plus_val_to_add") - - online_features_dict = get_online_features_dict( - environment=environment, - endpoint=feature_server_endpoint, - features=feature_refs, - entity_rows=entity_rows, - full_feature_names=full_feature_names, - ) - - # Test that the on demand feature views compute properly even if the dependent conv_rate - # feature isn't requested. - online_features_no_conv_rate = get_online_features_dict( - environment=environment, - endpoint=feature_server_endpoint, - features=[ref for ref in feature_refs if ref != "driver_stats:conv_rate"], - entity_rows=entity_rows, - full_feature_names=full_feature_names, - ) - - assert online_features_no_conv_rate is not None - - keys = set(online_features_dict.keys()) - expected_keys = set( - f.replace(":", "__") if full_feature_names else f.split(":")[-1] - for f in feature_refs - ) | {"customer_id", "driver_id"} - assert ( - keys == expected_keys - ), f"Response keys are different from expected: {keys - expected_keys} (extra) and {expected_keys - keys} (missing)" - - tc = unittest.TestCase() - for i, entity_row in enumerate(entity_rows): - df_features = get_latest_feature_values_from_dataframes( - driver_df=drivers_df, - customer_df=customers_df, - orders_df=orders_df, - global_df=global_df, - entity_row=entity_row, - ) - - assert df_features["customer_id"] == online_features_dict["customer_id"][i] - assert df_features["driver_id"] == online_features_dict["driver_id"][i] - tc.assertAlmostEqual( - online_features_dict[ - response_feature_name( - "conv_rate_plus_100", feature_refs, full_feature_names - ) - ][i], - df_features["conv_rate"] + 100, - delta=0.0001, - ) - tc.assertAlmostEqual( - online_features_dict[ - response_feature_name( - "conv_rate_plus_val_to_add", feature_refs, full_feature_names - ) - ][i], - df_features["conv_rate"] + df_features["val_to_add"], - delta=0.0001, - ) - for unprefixed_feature_ref in unprefixed_feature_refs: - tc.assertAlmostEqual( - df_features[unprefixed_feature_ref], - online_features_dict[ - response_feature_name( - unprefixed_feature_ref, feature_refs, full_feature_names - ) - ][i], - delta=0.0001, - ) - - # Check what happens for missing values - missing_responses_dict = get_online_features_dict( - environment=environment, - endpoint=feature_server_endpoint, - features=feature_refs, - entity_rows=[{"driver_id": 0, "customer_id": 0, "val_to_add": 100}], - full_feature_names=full_feature_names, - ) - assert missing_responses_dict is not None - for unprefixed_feature_ref in unprefixed_feature_refs: - if unprefixed_feature_ref not in {"num_rides", "avg_ride_length"}: - tc.assertIsNone( - missing_responses_dict[ - response_feature_name( - unprefixed_feature_ref, feature_refs, full_feature_names - ) - ][0] - ) - - # Check what happens for missing request data - with pytest.raises(RequestDataNotFoundInEntityRowsException): - get_online_features_dict( - environment=environment, - endpoint=feature_server_endpoint, - features=feature_refs, - entity_rows=[{"driver_id": 0, "customer_id": 0}], - full_feature_names=full_feature_names, - ) - - assert_feature_service_correctness( - environment, - feature_server_endpoint, - feature_service, - entity_rows, - full_feature_names, - drivers_df, - customers_df, - orders_df, - global_df, - ) - - entity_rows = [ - {"origin_id": origin, "destination_id": destination} - for (_driver, _customer, origin, destination) in zip( - sample_drivers, sample_customers, *sample_location_pairs - ) - ] - assert_feature_service_entity_mapping_correctness( - environment, - feature_server_endpoint, - feature_service_entity_mapping, - entity_rows, - full_feature_names, - origins_df, - destinations_df, - ) - - -@pytest.mark.integration -@pytest.mark.universal_online_stores(only=["redis"]) -def test_online_store_cleanup(environment, universal_data_sources): - """ - Some online store implementations (like Redis) keep features from different features views - but with common entities together. - This might end up with deletion of all features attached to the entity, - when only one feature view was deletion target (see https://github.com/feast-dev/feast/issues/2150). - - Plan: - 1. Register two feature views with common entity "driver" - 2. Materialize data - 3. Check if features are available (via online retrieval) - 4. Delete one feature view - 5. Check that features for other are still available - 6. Delete another feature view (and create again) - 7. Verify that features for both feature view were deleted - """ - fs = environment.feature_store - entities, datasets, data_sources = universal_data_sources - driver_stats_fv = construct_universal_feature_views(data_sources).driver - - driver_entities = entities.driver_vals - df = pd.DataFrame( - { - "ts_1": [environment.end_date] * len(driver_entities), - "created_ts": [environment.end_date] * len(driver_entities), - "driver_id": driver_entities, - "value": np.random.random(size=len(driver_entities)), - } - ) - - ds = environment.data_source_creator.create_data_source( - df, destination_name="simple_driver_dataset" - ) - - simple_driver_fv = driver_feature_view( - data_source=ds, name="test_universal_online_simple_driver" - ) - - fs.apply([driver(), simple_driver_fv, driver_stats_fv]) - - fs.materialize( - environment.start_date - timedelta(days=1), - environment.end_date + timedelta(days=1), - ) - expected_values = df.sort_values(by="driver_id") - - features = [f"{simple_driver_fv.name}:value"] - entity_rows = [{"driver_id": driver_id} for driver_id in sorted(driver_entities)] - - online_features = fs.get_online_features( - features=features, entity_rows=entity_rows - ).to_dict() - assert np.allclose(expected_values["value"], online_features["value"]) - - fs.apply( - objects=[simple_driver_fv], objects_to_delete=[driver_stats_fv], partial=False - ) - - online_features = fs.get_online_features( - features=features, entity_rows=entity_rows - ).to_dict() - assert np.allclose(expected_values["value"], online_features["value"]) - - fs.apply(objects=[], objects_to_delete=[simple_driver_fv], partial=False) - - def eventually_apply() -> Tuple[None, bool]: - try: - fs.apply([simple_driver_fv]) - except BotoCoreError: - return None, False - - return None, True - - # Online store backend might have eventual consistency in schema update - # So recreating table that was just deleted might need some retries - wait_retry_backoff(eventually_apply, timeout_secs=60) - - online_features = fs.get_online_features( - features=features, entity_rows=entity_rows - ).to_dict() - assert all(v is None for v in online_features["value"]) - - -def response_feature_name( - feature: str, feature_refs: List[str], full_feature_names: bool -) -> str: - if not full_feature_names: - return feature - - for feature_ref in feature_refs: - if feature_ref.endswith(feature): - return feature_ref.replace(":", "__") - - return feature - - -def get_latest_row(entity_row, df, join_key, entity_key): - rows = df[df[join_key] == entity_row[entity_key]] - return rows.loc[rows["event_timestamp"].idxmax()].to_dict() - - -def get_latest_feature_values_from_dataframes( - driver_df, - customer_df, - orders_df, - entity_row, - global_df=None, - origin_df=None, - destination_df=None, -): - latest_driver_row = get_latest_row(entity_row, driver_df, "driver_id", "driver_id") - latest_customer_row = get_latest_row( - entity_row, customer_df, "customer_id", "customer_id" - ) - - # Since the event timestamp columns may contain timestamps of different timezones, - # we must first convert the timestamps to UTC before we can compare them. - order_rows = orders_df[ - (orders_df["driver_id"] == entity_row["driver_id"]) - & (orders_df["customer_id"] == entity_row["customer_id"]) - ] - timestamps = order_rows[["event_timestamp"]] - timestamps["event_timestamp"] = pd.to_datetime( - timestamps["event_timestamp"], utc=True - ) - max_index = timestamps["event_timestamp"].idxmax() - latest_orders_row = order_rows.loc[max_index] - - if global_df is not None: - latest_global_row = global_df.loc[ - global_df["event_timestamp"].idxmax() - ].to_dict() - if origin_df is not None: - latest_location_row = get_latest_feature_values_for_location_df( - entity_row, origin_df, destination_df - ) - - request_data_features = entity_row.copy() - request_data_features.pop("driver_id") - request_data_features.pop("customer_id") - if global_df is not None: - return { - **latest_customer_row, - **latest_driver_row, - **latest_orders_row, - **latest_global_row, - **request_data_features, - } - if origin_df is not None: - request_data_features.pop("origin_id") - request_data_features.pop("destination_id") - return { - **latest_customer_row, - **latest_driver_row, - **latest_orders_row, - **latest_location_row, - **request_data_features, - } - return { - **latest_customer_row, - **latest_driver_row, - **latest_orders_row, - **request_data_features, - } - - -def get_latest_feature_values_for_location_df(entity_row, origin_df, destination_df): - latest_origin_row = get_latest_row( - entity_row, origin_df, "location_id", "origin_id" - ) - latest_destination_row = get_latest_row( - entity_row, destination_df, "location_id", "destination_id" - ) - # Need full feature names for shadow entities - latest_origin_row["origin__temperature"] = latest_origin_row.pop("temperature") - latest_destination_row["destination__temperature"] = latest_destination_row.pop( - "temperature" - ) - - return { - **latest_origin_row, - **latest_destination_row, - } - - -def get_latest_feature_values_from_location_df(entity_row, location_df): - return get_latest_row(entity_row, location_df, "location_id", "location_id") - - -def assert_feature_service_correctness( - environment, - endpoint, - feature_service, - entity_rows, - full_feature_names, - drivers_df, - customers_df, - orders_df, - global_df, -): - feature_service_online_features_dict = get_online_features_dict( - environment=environment, - endpoint=endpoint, - features=feature_service, - entity_rows=entity_rows, - full_feature_names=full_feature_names, - ) - feature_service_keys = feature_service_online_features_dict.keys() - expected_feature_refs = [ - f"{projection.name_to_use()}__{feature.name}" - if full_feature_names - else feature.name - for projection in feature_service.feature_view_projections - for feature in projection.features - ] - assert set(feature_service_keys) == set(expected_feature_refs) | { - "customer_id", - "driver_id", - } - - tc = unittest.TestCase() - for i, entity_row in enumerate(entity_rows): - df_features = get_latest_feature_values_from_dataframes( - driver_df=drivers_df, - customer_df=customers_df, - orders_df=orders_df, - global_df=global_df, - entity_row=entity_row, - ) - tc.assertAlmostEqual( - feature_service_online_features_dict[ - response_feature_name( - "conv_rate_plus_100", expected_feature_refs, full_feature_names - ) - ][i], - df_features["conv_rate"] + 100, - delta=0.0001, - ) - - -def assert_feature_service_entity_mapping_correctness( - environment, - endpoint, - feature_service, - entity_rows, - full_feature_names, - origins_df, - destinations_df, -): - if full_feature_names: - feature_service_online_features_dict = get_online_features_dict( - environment=environment, - endpoint=endpoint, - features=feature_service, - entity_rows=entity_rows, - full_feature_names=full_feature_names, - ) - feature_service_keys = feature_service_online_features_dict.keys() - - expected_features = [ - f"{projection.name_to_use()}__{feature.name}" - if full_feature_names - else feature.name - for projection in feature_service.feature_view_projections - for feature in projection.features - ] - assert set(feature_service_keys) == set(expected_features) | { - "destination_id", - "origin_id", - } - - for i, entity_row in enumerate(entity_rows): - df_features = get_latest_feature_values_for_location_df( - origin_df=origins_df, - destination_df=destinations_df, - entity_row=entity_row, - ) - for feature_name in ["origin__temperature", "destination__temperature"]: - assert ( - feature_service_online_features_dict[feature_name][i] - == df_features[feature_name] - ) - else: - # using 2 of the same FeatureView without full_feature_names=True will result in collision - with pytest.raises(FeatureNameCollisionError): - get_online_features_dict( - environment=environment, - endpoint=endpoint, - features=feature_service, - entity_rows=entity_rows, - full_feature_names=full_feature_names, - ) +# @pytest.mark.integration +# @pytest.mark.universal_online_stores +# # @pytest.mark.goserver Disabling because the go fs tests are flaking in CI. TODO(achals): uncomment after fixed. +# @pytest.mark.parametrize("full_feature_names", [True, False], ids=lambda v: str(v)) +# def test_stream_feature_view_online_retrieval( +# environment, universal_data_sources, feature_server_endpoint, full_feature_names +# ): +# """ +# Tests materialization and online retrieval for stream feature views. + +# This test is separate from test_online_retrieval since combining feature views and +# stream feature views into a single test resulted in test flakiness. This is tech +# debt that should be resolved soon. +# """ +# # Set up feature store. +# fs = environment.feature_store +# entities, datasets, data_sources = universal_data_sources +# feature_views = construct_universal_feature_views(data_sources) +# pushable_feature_view = feature_views.pushed_locations +# fs.apply([location(), pushable_feature_view]) + +# # Materialize. +# fs.materialize( +# environment.start_date - timedelta(days=1), +# environment.end_date + timedelta(days=1), +# ) + +# # Get online features by randomly sampling 10 entities that exist in the batch source. +# sample_locations = datasets.location_df.sample(10)["location_id"] +# entity_rows = [ +# {"location_id": sample_location} for sample_location in sample_locations +# ] + +# feature_refs = [ +# "pushable_location_stats:temperature", +# ] +# unprefixed_feature_refs = [f.rsplit(":", 1)[-1] for f in feature_refs if ":" in f] + +# online_features_dict = get_online_features_dict( +# environment=environment, +# endpoint=feature_server_endpoint, +# features=feature_refs, +# entity_rows=entity_rows, +# full_feature_names=full_feature_names, +# ) + +# # Check that the response has the expected set of keys. +# keys = set(online_features_dict.keys()) +# expected_keys = set( +# f.replace(":", "__") if full_feature_names else f.split(":")[-1] +# for f in feature_refs +# ) | {"location_id"} +# assert ( +# keys == expected_keys +# ), f"Response keys are different from expected: {keys - expected_keys} (extra) and {expected_keys - keys} (missing)" + +# # Check that the feature values match. +# tc = unittest.TestCase() +# for i, entity_row in enumerate(entity_rows): +# df_features = get_latest_feature_values_from_location_df( +# entity_row, datasets.location_df +# ) + +# assert df_features["location_id"] == online_features_dict["location_id"][i] +# for unprefixed_feature_ref in unprefixed_feature_refs: +# tc.assertAlmostEqual( +# df_features[unprefixed_feature_ref], +# online_features_dict[ +# response_feature_name( +# unprefixed_feature_ref, feature_refs, full_feature_names +# ) +# ][i], +# delta=0.0001, +# ) + + +# @pytest.mark.integration +# @pytest.mark.universal_online_stores +# # @pytest.mark.goserver Disabling because the go fs tests are flaking in CI. TODO(achals): uncomment after fixed. +# @pytest.mark.parametrize("full_feature_names", [True, False], ids=lambda v: str(v)) +# def test_online_retrieval( +# environment, universal_data_sources, feature_server_endpoint, full_feature_names +# ): +# fs = environment.feature_store +# entities, datasets, data_sources = universal_data_sources +# feature_views = construct_universal_feature_views(data_sources) + +# feature_service = FeatureService( +# "convrate_plus100", +# features=[ +# feature_views.driver[["conv_rate"]], +# feature_views.driver_odfv, +# feature_views.customer[["current_balance"]], +# ], +# ) +# feature_service_entity_mapping = FeatureService( +# name="entity_mapping", +# features=[ +# feature_views.location.with_name("origin").with_join_key_map( +# {"location_id": "origin_id"} +# ), +# feature_views.location.with_name("destination").with_join_key_map( +# {"location_id": "destination_id"} +# ), +# ], +# ) + +# feast_objects = [] +# feast_objects.extend(feature_views.values()) +# feast_objects.extend( +# [ +# driver(), +# customer(), +# location(), +# feature_service, +# feature_service_entity_mapping, +# ] +# ) +# fs.apply(feast_objects) +# fs.materialize( +# environment.start_date - timedelta(days=1), +# environment.end_date + timedelta(days=1), +# ) + +# entity_sample = datasets.orders_df.sample(10)[ +# ["customer_id", "driver_id", "order_id", "event_timestamp"] +# ] +# orders_df = datasets.orders_df[ +# ( +# datasets.orders_df["customer_id"].isin(entity_sample["customer_id"]) +# & datasets.orders_df["driver_id"].isin(entity_sample["driver_id"]) +# ) +# ] + +# sample_drivers = entity_sample["driver_id"] +# drivers_df = datasets.driver_df[ +# datasets.driver_df["driver_id"].isin(sample_drivers) +# ] + +# sample_customers = entity_sample["customer_id"] +# customers_df = datasets.customer_df[ +# datasets.customer_df["customer_id"].isin(sample_customers) +# ] + +# location_pairs = np.array(list(itertools.permutations(entities.location_vals, 2))) +# sample_location_pairs = location_pairs[ +# np.random.choice(len(location_pairs), 10) +# ].T.tolist() +# origins_df = datasets.location_df[ +# datasets.location_df["location_id"].isin(sample_location_pairs[0]) +# ] +# destinations_df = datasets.location_df[ +# datasets.location_df["location_id"].isin(sample_location_pairs[1]) +# ] + +# global_df = datasets.global_df + +# entity_rows = [ +# {"driver_id": d, "customer_id": c, "val_to_add": 50} +# for (d, c) in zip(sample_drivers, sample_customers) +# ] + +# feature_refs = [ +# "driver_stats:conv_rate", +# "driver_stats:avg_daily_trips", +# "customer_profile:current_balance", +# "customer_profile:avg_passenger_count", +# "customer_profile:lifetime_trip_count", +# "conv_rate_plus_100:conv_rate_plus_100", +# "conv_rate_plus_100:conv_rate_plus_val_to_add", +# "order:order_is_success", +# "global_stats:num_rides", +# "global_stats:avg_ride_length", +# ] +# unprefixed_feature_refs = [f.rsplit(":", 1)[-1] for f in feature_refs if ":" in f] +# # Remove the on demand feature view output features, since they're not present in the source dataframe +# unprefixed_feature_refs.remove("conv_rate_plus_100") +# unprefixed_feature_refs.remove("conv_rate_plus_val_to_add") + +# online_features_dict = get_online_features_dict( +# environment=environment, +# endpoint=feature_server_endpoint, +# features=feature_refs, +# entity_rows=entity_rows, +# full_feature_names=full_feature_names, +# ) + +# # Test that the on demand feature views compute properly even if the dependent conv_rate +# # feature isn't requested. +# online_features_no_conv_rate = get_online_features_dict( +# environment=environment, +# endpoint=feature_server_endpoint, +# features=[ref for ref in feature_refs if ref != "driver_stats:conv_rate"], +# entity_rows=entity_rows, +# full_feature_names=full_feature_names, +# ) + +# assert online_features_no_conv_rate is not None + +# keys = set(online_features_dict.keys()) +# expected_keys = set( +# f.replace(":", "__") if full_feature_names else f.split(":")[-1] +# for f in feature_refs +# ) | {"customer_id", "driver_id"} +# assert ( +# keys == expected_keys +# ), f"Response keys are different from expected: {keys - expected_keys} (extra) and {expected_keys - keys} (missing)" + +# tc = unittest.TestCase() +# for i, entity_row in enumerate(entity_rows): +# df_features = get_latest_feature_values_from_dataframes( +# driver_df=drivers_df, +# customer_df=customers_df, +# orders_df=orders_df, +# global_df=global_df, +# entity_row=entity_row, +# ) + +# assert df_features["customer_id"] == online_features_dict["customer_id"][i] +# assert df_features["driver_id"] == online_features_dict["driver_id"][i] +# tc.assertAlmostEqual( +# online_features_dict[ +# response_feature_name( +# "conv_rate_plus_100", feature_refs, full_feature_names +# ) +# ][i], +# df_features["conv_rate"] + 100, +# delta=0.0001, +# ) +# tc.assertAlmostEqual( +# online_features_dict[ +# response_feature_name( +# "conv_rate_plus_val_to_add", feature_refs, full_feature_names +# ) +# ][i], +# df_features["conv_rate"] + df_features["val_to_add"], +# delta=0.0001, +# ) +# for unprefixed_feature_ref in unprefixed_feature_refs: +# tc.assertAlmostEqual( +# df_features[unprefixed_feature_ref], +# online_features_dict[ +# response_feature_name( +# unprefixed_feature_ref, feature_refs, full_feature_names +# ) +# ][i], +# delta=0.0001, +# ) + +# # Check what happens for missing values +# missing_responses_dict = get_online_features_dict( +# environment=environment, +# endpoint=feature_server_endpoint, +# features=feature_refs, +# entity_rows=[{"driver_id": 0, "customer_id": 0, "val_to_add": 100}], +# full_feature_names=full_feature_names, +# ) +# assert missing_responses_dict is not None +# for unprefixed_feature_ref in unprefixed_feature_refs: +# if unprefixed_feature_ref not in {"num_rides", "avg_ride_length"}: +# tc.assertIsNone( +# missing_responses_dict[ +# response_feature_name( +# unprefixed_feature_ref, feature_refs, full_feature_names +# ) +# ][0] +# ) + +# # Check what happens for missing request data +# with pytest.raises(RequestDataNotFoundInEntityRowsException): +# get_online_features_dict( +# environment=environment, +# endpoint=feature_server_endpoint, +# features=feature_refs, +# entity_rows=[{"driver_id": 0, "customer_id": 0}], +# full_feature_names=full_feature_names, +# ) + +# assert_feature_service_correctness( +# environment, +# feature_server_endpoint, +# feature_service, +# entity_rows, +# full_feature_names, +# drivers_df, +# customers_df, +# orders_df, +# global_df, +# ) + +# entity_rows = [ +# {"origin_id": origin, "destination_id": destination} +# for (_driver, _customer, origin, destination) in zip( +# sample_drivers, sample_customers, *sample_location_pairs +# ) +# ] +# assert_feature_service_entity_mapping_correctness( +# environment, +# feature_server_endpoint, +# feature_service_entity_mapping, +# entity_rows, +# full_feature_names, +# origins_df, +# destinations_df, +# ) + + +# @pytest.mark.integration +# @pytest.mark.universal_online_stores(only=["redis"]) +# def test_online_store_cleanup(environment, universal_data_sources): +# """ +# Some online store implementations (like Redis) keep features from different features views +# but with common entities together. +# This might end up with deletion of all features attached to the entity, +# when only one feature view was deletion target (see https://github.com/feast-dev/feast/issues/2150). + +# Plan: +# 1. Register two feature views with common entity "driver" +# 2. Materialize data +# 3. Check if features are available (via online retrieval) +# 4. Delete one feature view +# 5. Check that features for other are still available +# 6. Delete another feature view (and create again) +# 7. Verify that features for both feature view were deleted +# """ +# fs = environment.feature_store +# entities, datasets, data_sources = universal_data_sources +# driver_stats_fv = construct_universal_feature_views(data_sources).driver + +# driver_entities = entities.driver_vals +# df = pd.DataFrame( +# { +# "ts_1": [environment.end_date] * len(driver_entities), +# "created_ts": [environment.end_date] * len(driver_entities), +# "driver_id": driver_entities, +# "value": np.random.random(size=len(driver_entities)), +# } +# ) + +# ds = environment.data_source_creator.create_data_source( +# df, destination_name="simple_driver_dataset" +# ) + +# simple_driver_fv = driver_feature_view( +# data_source=ds, name="test_universal_online_simple_driver" +# ) + +# fs.apply([driver(), simple_driver_fv, driver_stats_fv]) + +# fs.materialize( +# environment.start_date - timedelta(days=1), +# environment.end_date + timedelta(days=1), +# ) +# expected_values = df.sort_values(by="driver_id") + +# features = [f"{simple_driver_fv.name}:value"] +# entity_rows = [{"driver_id": driver_id} for driver_id in sorted(driver_entities)] + +# online_features = fs.get_online_features( +# features=features, entity_rows=entity_rows +# ).to_dict() +# assert np.allclose(expected_values["value"], online_features["value"]) + +# fs.apply( +# objects=[simple_driver_fv], objects_to_delete=[driver_stats_fv], partial=False +# ) + +# online_features = fs.get_online_features( +# features=features, entity_rows=entity_rows +# ).to_dict() +# assert np.allclose(expected_values["value"], online_features["value"]) + +# fs.apply(objects=[], objects_to_delete=[simple_driver_fv], partial=False) + +# def eventually_apply() -> Tuple[None, bool]: +# try: +# fs.apply([simple_driver_fv]) +# except BotoCoreError: +# return None, False + +# return None, True + +# # Online store backend might have eventual consistency in schema update +# # So recreating table that was just deleted might need some retries +# wait_retry_backoff(eventually_apply, timeout_secs=60) + +# online_features = fs.get_online_features( +# features=features, entity_rows=entity_rows +# ).to_dict() +# assert all(v is None for v in online_features["value"]) + + +# def response_feature_name( +# feature: str, feature_refs: List[str], full_feature_names: bool +# ) -> str: +# if not full_feature_names: +# return feature + +# for feature_ref in feature_refs: +# if feature_ref.endswith(feature): +# return feature_ref.replace(":", "__") + +# return feature + + +# def get_latest_row(entity_row, df, join_key, entity_key): +# rows = df[df[join_key] == entity_row[entity_key]] +# return rows.loc[rows["event_timestamp"].idxmax()].to_dict() + + +# def get_latest_feature_values_from_dataframes( +# driver_df, +# customer_df, +# orders_df, +# entity_row, +# global_df=None, +# origin_df=None, +# destination_df=None, +# ): +# latest_driver_row = get_latest_row(entity_row, driver_df, "driver_id", "driver_id") +# latest_customer_row = get_latest_row( +# entity_row, customer_df, "customer_id", "customer_id" +# ) + +# # Since the event timestamp columns may contain timestamps of different timezones, +# # we must first convert the timestamps to UTC before we can compare them. +# order_rows = orders_df[ +# (orders_df["driver_id"] == entity_row["driver_id"]) +# & (orders_df["customer_id"] == entity_row["customer_id"]) +# ] +# timestamps = order_rows[["event_timestamp"]] +# timestamps["event_timestamp"] = pd.to_datetime( +# timestamps["event_timestamp"], utc=True +# ) +# max_index = timestamps["event_timestamp"].idxmax() +# latest_orders_row = order_rows.loc[max_index] + +# if global_df is not None: +# latest_global_row = global_df.loc[ +# global_df["event_timestamp"].idxmax() +# ].to_dict() +# if origin_df is not None: +# latest_location_row = get_latest_feature_values_for_location_df( +# entity_row, origin_df, destination_df +# ) + +# request_data_features = entity_row.copy() +# request_data_features.pop("driver_id") +# request_data_features.pop("customer_id") +# if global_df is not None: +# return { +# **latest_customer_row, +# **latest_driver_row, +# **latest_orders_row, +# **latest_global_row, +# **request_data_features, +# } +# if origin_df is not None: +# request_data_features.pop("origin_id") +# request_data_features.pop("destination_id") +# return { +# **latest_customer_row, +# **latest_driver_row, +# **latest_orders_row, +# **latest_location_row, +# **request_data_features, +# } +# return { +# **latest_customer_row, +# **latest_driver_row, +# **latest_orders_row, +# **request_data_features, +# } + + +# def get_latest_feature_values_for_location_df(entity_row, origin_df, destination_df): +# latest_origin_row = get_latest_row( +# entity_row, origin_df, "location_id", "origin_id" +# ) +# latest_destination_row = get_latest_row( +# entity_row, destination_df, "location_id", "destination_id" +# ) +# # Need full feature names for shadow entities +# latest_origin_row["origin__temperature"] = latest_origin_row.pop("temperature") +# latest_destination_row["destination__temperature"] = latest_destination_row.pop( +# "temperature" +# ) + +# return { +# **latest_origin_row, +# **latest_destination_row, +# } + + +# def get_latest_feature_values_from_location_df(entity_row, location_df): +# return get_latest_row(entity_row, location_df, "location_id", "location_id") + + +# def assert_feature_service_correctness( +# environment, +# endpoint, +# feature_service, +# entity_rows, +# full_feature_names, +# drivers_df, +# customers_df, +# orders_df, +# global_df, +# ): +# feature_service_online_features_dict = get_online_features_dict( +# environment=environment, +# endpoint=endpoint, +# features=feature_service, +# entity_rows=entity_rows, +# full_feature_names=full_feature_names, +# ) +# feature_service_keys = feature_service_online_features_dict.keys() +# expected_feature_refs = [ +# f"{projection.name_to_use()}__{feature.name}" +# if full_feature_names +# else feature.name +# for projection in feature_service.feature_view_projections +# for feature in projection.features +# ] +# assert set(feature_service_keys) == set(expected_feature_refs) | { +# "customer_id", +# "driver_id", +# } + +# tc = unittest.TestCase() +# for i, entity_row in enumerate(entity_rows): +# df_features = get_latest_feature_values_from_dataframes( +# driver_df=drivers_df, +# customer_df=customers_df, +# orders_df=orders_df, +# global_df=global_df, +# entity_row=entity_row, +# ) +# tc.assertAlmostEqual( +# feature_service_online_features_dict[ +# response_feature_name( +# "conv_rate_plus_100", expected_feature_refs, full_feature_names +# ) +# ][i], +# df_features["conv_rate"] + 100, +# delta=0.0001, +# ) + + +# def assert_feature_service_entity_mapping_correctness( +# environment, +# endpoint, +# feature_service, +# entity_rows, +# full_feature_names, +# origins_df, +# destinations_df, +# ): +# if full_feature_names: +# feature_service_online_features_dict = get_online_features_dict( +# environment=environment, +# endpoint=endpoint, +# features=feature_service, +# entity_rows=entity_rows, +# full_feature_names=full_feature_names, +# ) +# feature_service_keys = feature_service_online_features_dict.keys() + +# expected_features = [ +# f"{projection.name_to_use()}__{feature.name}" +# if full_feature_names +# else feature.name +# for projection in feature_service.feature_view_projections +# for feature in projection.features +# ] +# assert set(feature_service_keys) == set(expected_features) | { +# "destination_id", +# "origin_id", +# } + +# for i, entity_row in enumerate(entity_rows): +# df_features = get_latest_feature_values_for_location_df( +# origin_df=origins_df, +# destination_df=destinations_df, +# entity_row=entity_row, +# ) +# for feature_name in ["origin__temperature", "destination__temperature"]: +# assert ( +# feature_service_online_features_dict[feature_name][i] +# == df_features[feature_name] +# ) +# else: +# # using 2 of the same FeatureView without full_feature_names=True will result in collision +# with pytest.raises(FeatureNameCollisionError): +# get_online_features_dict( +# environment=environment, +# endpoint=endpoint, +# features=feature_service, +# entity_rows=entity_rows, +# full_feature_names=full_feature_names, +# ) From ce1092184561e1026d7972a86c34ba384e458e92 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 21 Jun 2022 14:46:10 -0700 Subject: [PATCH 20/26] Fix test Signed-off-by: Kevin Zhang --- .../online_store/test_universal_online.py | 960 +++++++++--------- 1 file changed, 480 insertions(+), 480 deletions(-) diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index 3d066e7ba70..9d4db3e03e5 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -517,528 +517,528 @@ def test_online_retrieval_with_event_timestamps( # ) -# @pytest.mark.integration -# @pytest.mark.universal_online_stores -# # @pytest.mark.goserver Disabling because the go fs tests are flaking in CI. TODO(achals): uncomment after fixed. -# @pytest.mark.parametrize("full_feature_names", [True, False], ids=lambda v: str(v)) -# def test_online_retrieval( -# environment, universal_data_sources, feature_server_endpoint, full_feature_names -# ): -# fs = environment.feature_store -# entities, datasets, data_sources = universal_data_sources -# feature_views = construct_universal_feature_views(data_sources) - -# feature_service = FeatureService( -# "convrate_plus100", -# features=[ -# feature_views.driver[["conv_rate"]], -# feature_views.driver_odfv, -# feature_views.customer[["current_balance"]], -# ], -# ) -# feature_service_entity_mapping = FeatureService( -# name="entity_mapping", -# features=[ -# feature_views.location.with_name("origin").with_join_key_map( -# {"location_id": "origin_id"} -# ), -# feature_views.location.with_name("destination").with_join_key_map( -# {"location_id": "destination_id"} -# ), -# ], -# ) - -# feast_objects = [] -# feast_objects.extend(feature_views.values()) -# feast_objects.extend( -# [ -# driver(), -# customer(), -# location(), -# feature_service, -# feature_service_entity_mapping, -# ] -# ) -# fs.apply(feast_objects) -# fs.materialize( -# environment.start_date - timedelta(days=1), -# environment.end_date + timedelta(days=1), -# ) - -# entity_sample = datasets.orders_df.sample(10)[ -# ["customer_id", "driver_id", "order_id", "event_timestamp"] -# ] -# orders_df = datasets.orders_df[ -# ( -# datasets.orders_df["customer_id"].isin(entity_sample["customer_id"]) -# & datasets.orders_df["driver_id"].isin(entity_sample["driver_id"]) -# ) -# ] - -# sample_drivers = entity_sample["driver_id"] -# drivers_df = datasets.driver_df[ -# datasets.driver_df["driver_id"].isin(sample_drivers) -# ] - -# sample_customers = entity_sample["customer_id"] -# customers_df = datasets.customer_df[ -# datasets.customer_df["customer_id"].isin(sample_customers) -# ] - -# location_pairs = np.array(list(itertools.permutations(entities.location_vals, 2))) -# sample_location_pairs = location_pairs[ -# np.random.choice(len(location_pairs), 10) -# ].T.tolist() -# origins_df = datasets.location_df[ -# datasets.location_df["location_id"].isin(sample_location_pairs[0]) -# ] -# destinations_df = datasets.location_df[ -# datasets.location_df["location_id"].isin(sample_location_pairs[1]) -# ] - -# global_df = datasets.global_df +@pytest.mark.integration +@pytest.mark.universal_online_stores +# @pytest.mark.goserver Disabling because the go fs tests are flaking in CI. TODO(achals): uncomment after fixed. +@pytest.mark.parametrize("full_feature_names", [True, False], ids=lambda v: str(v)) +def test_online_retrieval( + environment, universal_data_sources, feature_server_endpoint, full_feature_names +): + fs = environment.feature_store + entities, datasets, data_sources = universal_data_sources + feature_views = construct_universal_feature_views(data_sources) -# entity_rows = [ -# {"driver_id": d, "customer_id": c, "val_to_add": 50} -# for (d, c) in zip(sample_drivers, sample_customers) -# ] + feature_service = FeatureService( + "convrate_plus100", + features=[ + feature_views.driver[["conv_rate"]], + feature_views.driver_odfv, + feature_views.customer[["current_balance"]], + ], + ) + feature_service_entity_mapping = FeatureService( + name="entity_mapping", + features=[ + feature_views.location.with_name("origin").with_join_key_map( + {"location_id": "origin_id"} + ), + feature_views.location.with_name("destination").with_join_key_map( + {"location_id": "destination_id"} + ), + ], + ) -# feature_refs = [ -# "driver_stats:conv_rate", -# "driver_stats:avg_daily_trips", -# "customer_profile:current_balance", -# "customer_profile:avg_passenger_count", -# "customer_profile:lifetime_trip_count", -# "conv_rate_plus_100:conv_rate_plus_100", -# "conv_rate_plus_100:conv_rate_plus_val_to_add", -# "order:order_is_success", -# "global_stats:num_rides", -# "global_stats:avg_ride_length", -# ] -# unprefixed_feature_refs = [f.rsplit(":", 1)[-1] for f in feature_refs if ":" in f] -# # Remove the on demand feature view output features, since they're not present in the source dataframe -# unprefixed_feature_refs.remove("conv_rate_plus_100") -# unprefixed_feature_refs.remove("conv_rate_plus_val_to_add") + feast_objects = [] + feast_objects.extend(feature_views.values()) + feast_objects.extend( + [ + driver(), + customer(), + location(), + feature_service, + feature_service_entity_mapping, + ] + ) + fs.apply(feast_objects) + fs.materialize( + environment.start_date - timedelta(days=1), + environment.end_date + timedelta(days=1), + ) -# online_features_dict = get_online_features_dict( -# environment=environment, -# endpoint=feature_server_endpoint, -# features=feature_refs, -# entity_rows=entity_rows, -# full_feature_names=full_feature_names, -# ) + entity_sample = datasets.orders_df.sample(10)[ + ["customer_id", "driver_id", "order_id", "event_timestamp"] + ] + orders_df = datasets.orders_df[ + ( + datasets.orders_df["customer_id"].isin(entity_sample["customer_id"]) + & datasets.orders_df["driver_id"].isin(entity_sample["driver_id"]) + ) + ] + + sample_drivers = entity_sample["driver_id"] + drivers_df = datasets.driver_df[ + datasets.driver_df["driver_id"].isin(sample_drivers) + ] + + sample_customers = entity_sample["customer_id"] + customers_df = datasets.customer_df[ + datasets.customer_df["customer_id"].isin(sample_customers) + ] + + location_pairs = np.array(list(itertools.permutations(entities.location_vals, 2))) + sample_location_pairs = location_pairs[ + np.random.choice(len(location_pairs), 10) + ].T.tolist() + origins_df = datasets.location_df[ + datasets.location_df["location_id"].isin(sample_location_pairs[0]) + ] + destinations_df = datasets.location_df[ + datasets.location_df["location_id"].isin(sample_location_pairs[1]) + ] + + global_df = datasets.global_df + + entity_rows = [ + {"driver_id": d, "customer_id": c, "val_to_add": 50} + for (d, c) in zip(sample_drivers, sample_customers) + ] + + feature_refs = [ + "driver_stats:conv_rate", + "driver_stats:avg_daily_trips", + "customer_profile:current_balance", + "customer_profile:avg_passenger_count", + "customer_profile:lifetime_trip_count", + "conv_rate_plus_100:conv_rate_plus_100", + "conv_rate_plus_100:conv_rate_plus_val_to_add", + "order:order_is_success", + "global_stats:num_rides", + "global_stats:avg_ride_length", + ] + unprefixed_feature_refs = [f.rsplit(":", 1)[-1] for f in feature_refs if ":" in f] + # Remove the on demand feature view output features, since they're not present in the source dataframe + unprefixed_feature_refs.remove("conv_rate_plus_100") + unprefixed_feature_refs.remove("conv_rate_plus_val_to_add") + + online_features_dict = get_online_features_dict( + environment=environment, + endpoint=feature_server_endpoint, + features=feature_refs, + entity_rows=entity_rows, + full_feature_names=full_feature_names, + ) -# # Test that the on demand feature views compute properly even if the dependent conv_rate -# # feature isn't requested. -# online_features_no_conv_rate = get_online_features_dict( -# environment=environment, -# endpoint=feature_server_endpoint, -# features=[ref for ref in feature_refs if ref != "driver_stats:conv_rate"], -# entity_rows=entity_rows, -# full_feature_names=full_feature_names, -# ) + # Test that the on demand feature views compute properly even if the dependent conv_rate + # feature isn't requested. + online_features_no_conv_rate = get_online_features_dict( + environment=environment, + endpoint=feature_server_endpoint, + features=[ref for ref in feature_refs if ref != "driver_stats:conv_rate"], + entity_rows=entity_rows, + full_feature_names=full_feature_names, + ) -# assert online_features_no_conv_rate is not None + assert online_features_no_conv_rate is not None + + keys = set(online_features_dict.keys()) + expected_keys = set( + f.replace(":", "__") if full_feature_names else f.split(":")[-1] + for f in feature_refs + ) | {"customer_id", "driver_id"} + assert ( + keys == expected_keys + ), f"Response keys are different from expected: {keys - expected_keys} (extra) and {expected_keys - keys} (missing)" + + tc = unittest.TestCase() + for i, entity_row in enumerate(entity_rows): + df_features = get_latest_feature_values_from_dataframes( + driver_df=drivers_df, + customer_df=customers_df, + orders_df=orders_df, + global_df=global_df, + entity_row=entity_row, + ) -# keys = set(online_features_dict.keys()) -# expected_keys = set( -# f.replace(":", "__") if full_feature_names else f.split(":")[-1] -# for f in feature_refs -# ) | {"customer_id", "driver_id"} -# assert ( -# keys == expected_keys -# ), f"Response keys are different from expected: {keys - expected_keys} (extra) and {expected_keys - keys} (missing)" + assert df_features["customer_id"] == online_features_dict["customer_id"][i] + assert df_features["driver_id"] == online_features_dict["driver_id"][i] + tc.assertAlmostEqual( + online_features_dict[ + response_feature_name( + "conv_rate_plus_100", feature_refs, full_feature_names + ) + ][i], + df_features["conv_rate"] + 100, + delta=0.0001, + ) + tc.assertAlmostEqual( + online_features_dict[ + response_feature_name( + "conv_rate_plus_val_to_add", feature_refs, full_feature_names + ) + ][i], + df_features["conv_rate"] + df_features["val_to_add"], + delta=0.0001, + ) + for unprefixed_feature_ref in unprefixed_feature_refs: + tc.assertAlmostEqual( + df_features[unprefixed_feature_ref], + online_features_dict[ + response_feature_name( + unprefixed_feature_ref, feature_refs, full_feature_names + ) + ][i], + delta=0.0001, + ) + + # Check what happens for missing values + missing_responses_dict = get_online_features_dict( + environment=environment, + endpoint=feature_server_endpoint, + features=feature_refs, + entity_rows=[{"driver_id": 0, "customer_id": 0, "val_to_add": 100}], + full_feature_names=full_feature_names, + ) + assert missing_responses_dict is not None + for unprefixed_feature_ref in unprefixed_feature_refs: + if unprefixed_feature_ref not in {"num_rides", "avg_ride_length"}: + tc.assertIsNone( + missing_responses_dict[ + response_feature_name( + unprefixed_feature_ref, feature_refs, full_feature_names + ) + ][0] + ) + + # Check what happens for missing request data + with pytest.raises(RequestDataNotFoundInEntityRowsException): + get_online_features_dict( + environment=environment, + endpoint=feature_server_endpoint, + features=feature_refs, + entity_rows=[{"driver_id": 0, "customer_id": 0}], + full_feature_names=full_feature_names, + ) -# tc = unittest.TestCase() -# for i, entity_row in enumerate(entity_rows): -# df_features = get_latest_feature_values_from_dataframes( -# driver_df=drivers_df, -# customer_df=customers_df, -# orders_df=orders_df, -# global_df=global_df, -# entity_row=entity_row, -# ) + assert_feature_service_correctness( + environment, + feature_server_endpoint, + feature_service, + entity_rows, + full_feature_names, + drivers_df, + customers_df, + orders_df, + global_df, + ) -# assert df_features["customer_id"] == online_features_dict["customer_id"][i] -# assert df_features["driver_id"] == online_features_dict["driver_id"][i] -# tc.assertAlmostEqual( -# online_features_dict[ -# response_feature_name( -# "conv_rate_plus_100", feature_refs, full_feature_names -# ) -# ][i], -# df_features["conv_rate"] + 100, -# delta=0.0001, -# ) -# tc.assertAlmostEqual( -# online_features_dict[ -# response_feature_name( -# "conv_rate_plus_val_to_add", feature_refs, full_feature_names -# ) -# ][i], -# df_features["conv_rate"] + df_features["val_to_add"], -# delta=0.0001, -# ) -# for unprefixed_feature_ref in unprefixed_feature_refs: -# tc.assertAlmostEqual( -# df_features[unprefixed_feature_ref], -# online_features_dict[ -# response_feature_name( -# unprefixed_feature_ref, feature_refs, full_feature_names -# ) -# ][i], -# delta=0.0001, -# ) + entity_rows = [ + {"origin_id": origin, "destination_id": destination} + for (_driver, _customer, origin, destination) in zip( + sample_drivers, sample_customers, *sample_location_pairs + ) + ] + assert_feature_service_entity_mapping_correctness( + environment, + feature_server_endpoint, + feature_service_entity_mapping, + entity_rows, + full_feature_names, + origins_df, + destinations_df, + ) -# # Check what happens for missing values -# missing_responses_dict = get_online_features_dict( -# environment=environment, -# endpoint=feature_server_endpoint, -# features=feature_refs, -# entity_rows=[{"driver_id": 0, "customer_id": 0, "val_to_add": 100}], -# full_feature_names=full_feature_names, -# ) -# assert missing_responses_dict is not None -# for unprefixed_feature_ref in unprefixed_feature_refs: -# if unprefixed_feature_ref not in {"num_rides", "avg_ride_length"}: -# tc.assertIsNone( -# missing_responses_dict[ -# response_feature_name( -# unprefixed_feature_ref, feature_refs, full_feature_names -# ) -# ][0] -# ) -# # Check what happens for missing request data -# with pytest.raises(RequestDataNotFoundInEntityRowsException): -# get_online_features_dict( -# environment=environment, -# endpoint=feature_server_endpoint, -# features=feature_refs, -# entity_rows=[{"driver_id": 0, "customer_id": 0}], -# full_feature_names=full_feature_names, -# ) +@pytest.mark.integration +@pytest.mark.universal_online_stores(only=["redis"]) +def test_online_store_cleanup(environment, universal_data_sources): + """ + Some online store implementations (like Redis) keep features from different features views + but with common entities together. + This might end up with deletion of all features attached to the entity, + when only one feature view was deletion target (see https://github.com/feast-dev/feast/issues/2150). + + Plan: + 1. Register two feature views with common entity "driver" + 2. Materialize data + 3. Check if features are available (via online retrieval) + 4. Delete one feature view + 5. Check that features for other are still available + 6. Delete another feature view (and create again) + 7. Verify that features for both feature view were deleted + """ + fs = environment.feature_store + entities, datasets, data_sources = universal_data_sources + driver_stats_fv = construct_universal_feature_views(data_sources).driver -# assert_feature_service_correctness( -# environment, -# feature_server_endpoint, -# feature_service, -# entity_rows, -# full_feature_names, -# drivers_df, -# customers_df, -# orders_df, -# global_df, -# ) + driver_entities = entities.driver_vals + df = pd.DataFrame( + { + "ts_1": [environment.end_date] * len(driver_entities), + "created_ts": [environment.end_date] * len(driver_entities), + "driver_id": driver_entities, + "value": np.random.random(size=len(driver_entities)), + } + ) -# entity_rows = [ -# {"origin_id": origin, "destination_id": destination} -# for (_driver, _customer, origin, destination) in zip( -# sample_drivers, sample_customers, *sample_location_pairs -# ) -# ] -# assert_feature_service_entity_mapping_correctness( -# environment, -# feature_server_endpoint, -# feature_service_entity_mapping, -# entity_rows, -# full_feature_names, -# origins_df, -# destinations_df, -# ) + ds = environment.data_source_creator.create_data_source( + df, destination_name="simple_driver_dataset" + ) + simple_driver_fv = driver_feature_view( + data_source=ds, name="test_universal_online_simple_driver" + ) -# @pytest.mark.integration -# @pytest.mark.universal_online_stores(only=["redis"]) -# def test_online_store_cleanup(environment, universal_data_sources): -# """ -# Some online store implementations (like Redis) keep features from different features views -# but with common entities together. -# This might end up with deletion of all features attached to the entity, -# when only one feature view was deletion target (see https://github.com/feast-dev/feast/issues/2150). - -# Plan: -# 1. Register two feature views with common entity "driver" -# 2. Materialize data -# 3. Check if features are available (via online retrieval) -# 4. Delete one feature view -# 5. Check that features for other are still available -# 6. Delete another feature view (and create again) -# 7. Verify that features for both feature view were deleted -# """ -# fs = environment.feature_store -# entities, datasets, data_sources = universal_data_sources -# driver_stats_fv = construct_universal_feature_views(data_sources).driver - -# driver_entities = entities.driver_vals -# df = pd.DataFrame( -# { -# "ts_1": [environment.end_date] * len(driver_entities), -# "created_ts": [environment.end_date] * len(driver_entities), -# "driver_id": driver_entities, -# "value": np.random.random(size=len(driver_entities)), -# } -# ) + fs.apply([driver(), simple_driver_fv, driver_stats_fv]) -# ds = environment.data_source_creator.create_data_source( -# df, destination_name="simple_driver_dataset" -# ) + fs.materialize( + environment.start_date - timedelta(days=1), + environment.end_date + timedelta(days=1), + ) + expected_values = df.sort_values(by="driver_id") -# simple_driver_fv = driver_feature_view( -# data_source=ds, name="test_universal_online_simple_driver" -# ) + features = [f"{simple_driver_fv.name}:value"] + entity_rows = [{"driver_id": driver_id} for driver_id in sorted(driver_entities)] -# fs.apply([driver(), simple_driver_fv, driver_stats_fv]) + online_features = fs.get_online_features( + features=features, entity_rows=entity_rows + ).to_dict() + assert np.allclose(expected_values["value"], online_features["value"]) -# fs.materialize( -# environment.start_date - timedelta(days=1), -# environment.end_date + timedelta(days=1), -# ) -# expected_values = df.sort_values(by="driver_id") + fs.apply( + objects=[simple_driver_fv], objects_to_delete=[driver_stats_fv], partial=False + ) -# features = [f"{simple_driver_fv.name}:value"] -# entity_rows = [{"driver_id": driver_id} for driver_id in sorted(driver_entities)] + online_features = fs.get_online_features( + features=features, entity_rows=entity_rows + ).to_dict() + assert np.allclose(expected_values["value"], online_features["value"]) -# online_features = fs.get_online_features( -# features=features, entity_rows=entity_rows -# ).to_dict() -# assert np.allclose(expected_values["value"], online_features["value"]) + fs.apply(objects=[], objects_to_delete=[simple_driver_fv], partial=False) -# fs.apply( -# objects=[simple_driver_fv], objects_to_delete=[driver_stats_fv], partial=False -# ) + def eventually_apply() -> Tuple[None, bool]: + try: + fs.apply([simple_driver_fv]) + except BotoCoreError: + return None, False -# online_features = fs.get_online_features( -# features=features, entity_rows=entity_rows -# ).to_dict() -# assert np.allclose(expected_values["value"], online_features["value"]) + return None, True -# fs.apply(objects=[], objects_to_delete=[simple_driver_fv], partial=False) + # Online store backend might have eventual consistency in schema update + # So recreating table that was just deleted might need some retries + wait_retry_backoff(eventually_apply, timeout_secs=60) -# def eventually_apply() -> Tuple[None, bool]: -# try: -# fs.apply([simple_driver_fv]) -# except BotoCoreError: -# return None, False + online_features = fs.get_online_features( + features=features, entity_rows=entity_rows + ).to_dict() + assert all(v is None for v in online_features["value"]) -# return None, True -# # Online store backend might have eventual consistency in schema update -# # So recreating table that was just deleted might need some retries -# wait_retry_backoff(eventually_apply, timeout_secs=60) +def response_feature_name( + feature: str, feature_refs: List[str], full_feature_names: bool +) -> str: + if not full_feature_names: + return feature -# online_features = fs.get_online_features( -# features=features, entity_rows=entity_rows -# ).to_dict() -# assert all(v is None for v in online_features["value"]) + for feature_ref in feature_refs: + if feature_ref.endswith(feature): + return feature_ref.replace(":", "__") + return feature -# def response_feature_name( -# feature: str, feature_refs: List[str], full_feature_names: bool -# ) -> str: -# if not full_feature_names: -# return feature -# for feature_ref in feature_refs: -# if feature_ref.endswith(feature): -# return feature_ref.replace(":", "__") +def get_latest_row(entity_row, df, join_key, entity_key): + rows = df[df[join_key] == entity_row[entity_key]] + return rows.loc[rows["event_timestamp"].idxmax()].to_dict() -# return feature +def get_latest_feature_values_from_dataframes( + driver_df, + customer_df, + orders_df, + entity_row, + global_df=None, + origin_df=None, + destination_df=None, +): + latest_driver_row = get_latest_row(entity_row, driver_df, "driver_id", "driver_id") + latest_customer_row = get_latest_row( + entity_row, customer_df, "customer_id", "customer_id" + ) -# def get_latest_row(entity_row, df, join_key, entity_key): -# rows = df[df[join_key] == entity_row[entity_key]] -# return rows.loc[rows["event_timestamp"].idxmax()].to_dict() + # Since the event timestamp columns may contain timestamps of different timezones, + # we must first convert the timestamps to UTC before we can compare them. + order_rows = orders_df[ + (orders_df["driver_id"] == entity_row["driver_id"]) + & (orders_df["customer_id"] == entity_row["customer_id"]) + ] + timestamps = order_rows[["event_timestamp"]] + timestamps["event_timestamp"] = pd.to_datetime( + timestamps["event_timestamp"], utc=True + ) + max_index = timestamps["event_timestamp"].idxmax() + latest_orders_row = order_rows.loc[max_index] + + if global_df is not None: + latest_global_row = global_df.loc[ + global_df["event_timestamp"].idxmax() + ].to_dict() + if origin_df is not None: + latest_location_row = get_latest_feature_values_for_location_df( + entity_row, origin_df, destination_df + ) + request_data_features = entity_row.copy() + request_data_features.pop("driver_id") + request_data_features.pop("customer_id") + if global_df is not None: + return { + **latest_customer_row, + **latest_driver_row, + **latest_orders_row, + **latest_global_row, + **request_data_features, + } + if origin_df is not None: + request_data_features.pop("origin_id") + request_data_features.pop("destination_id") + return { + **latest_customer_row, + **latest_driver_row, + **latest_orders_row, + **latest_location_row, + **request_data_features, + } + return { + **latest_customer_row, + **latest_driver_row, + **latest_orders_row, + **request_data_features, + } -# def get_latest_feature_values_from_dataframes( -# driver_df, -# customer_df, -# orders_df, -# entity_row, -# global_df=None, -# origin_df=None, -# destination_df=None, -# ): -# latest_driver_row = get_latest_row(entity_row, driver_df, "driver_id", "driver_id") -# latest_customer_row = get_latest_row( -# entity_row, customer_df, "customer_id", "customer_id" -# ) -# # Since the event timestamp columns may contain timestamps of different timezones, -# # we must first convert the timestamps to UTC before we can compare them. -# order_rows = orders_df[ -# (orders_df["driver_id"] == entity_row["driver_id"]) -# & (orders_df["customer_id"] == entity_row["customer_id"]) -# ] -# timestamps = order_rows[["event_timestamp"]] -# timestamps["event_timestamp"] = pd.to_datetime( -# timestamps["event_timestamp"], utc=True -# ) -# max_index = timestamps["event_timestamp"].idxmax() -# latest_orders_row = order_rows.loc[max_index] - -# if global_df is not None: -# latest_global_row = global_df.loc[ -# global_df["event_timestamp"].idxmax() -# ].to_dict() -# if origin_df is not None: -# latest_location_row = get_latest_feature_values_for_location_df( -# entity_row, origin_df, destination_df -# ) +def get_latest_feature_values_for_location_df(entity_row, origin_df, destination_df): + latest_origin_row = get_latest_row( + entity_row, origin_df, "location_id", "origin_id" + ) + latest_destination_row = get_latest_row( + entity_row, destination_df, "location_id", "destination_id" + ) + # Need full feature names for shadow entities + latest_origin_row["origin__temperature"] = latest_origin_row.pop("temperature") + latest_destination_row["destination__temperature"] = latest_destination_row.pop( + "temperature" + ) -# request_data_features = entity_row.copy() -# request_data_features.pop("driver_id") -# request_data_features.pop("customer_id") -# if global_df is not None: -# return { -# **latest_customer_row, -# **latest_driver_row, -# **latest_orders_row, -# **latest_global_row, -# **request_data_features, -# } -# if origin_df is not None: -# request_data_features.pop("origin_id") -# request_data_features.pop("destination_id") -# return { -# **latest_customer_row, -# **latest_driver_row, -# **latest_orders_row, -# **latest_location_row, -# **request_data_features, -# } -# return { -# **latest_customer_row, -# **latest_driver_row, -# **latest_orders_row, -# **request_data_features, -# } - - -# def get_latest_feature_values_for_location_df(entity_row, origin_df, destination_df): -# latest_origin_row = get_latest_row( -# entity_row, origin_df, "location_id", "origin_id" -# ) -# latest_destination_row = get_latest_row( -# entity_row, destination_df, "location_id", "destination_id" -# ) -# # Need full feature names for shadow entities -# latest_origin_row["origin__temperature"] = latest_origin_row.pop("temperature") -# latest_destination_row["destination__temperature"] = latest_destination_row.pop( -# "temperature" -# ) + return { + **latest_origin_row, + **latest_destination_row, + } -# return { -# **latest_origin_row, -# **latest_destination_row, -# } +def get_latest_feature_values_from_location_df(entity_row, location_df): + return get_latest_row(entity_row, location_df, "location_id", "location_id") -# def get_latest_feature_values_from_location_df(entity_row, location_df): -# return get_latest_row(entity_row, location_df, "location_id", "location_id") +def assert_feature_service_correctness( + environment, + endpoint, + feature_service, + entity_rows, + full_feature_names, + drivers_df, + customers_df, + orders_df, + global_df, +): + feature_service_online_features_dict = get_online_features_dict( + environment=environment, + endpoint=endpoint, + features=feature_service, + entity_rows=entity_rows, + full_feature_names=full_feature_names, + ) + feature_service_keys = feature_service_online_features_dict.keys() + expected_feature_refs = [ + f"{projection.name_to_use()}__{feature.name}" + if full_feature_names + else feature.name + for projection in feature_service.feature_view_projections + for feature in projection.features + ] + assert set(feature_service_keys) == set(expected_feature_refs) | { + "customer_id", + "driver_id", + } -# def assert_feature_service_correctness( -# environment, -# endpoint, -# feature_service, -# entity_rows, -# full_feature_names, -# drivers_df, -# customers_df, -# orders_df, -# global_df, -# ): -# feature_service_online_features_dict = get_online_features_dict( -# environment=environment, -# endpoint=endpoint, -# features=feature_service, -# entity_rows=entity_rows, -# full_feature_names=full_feature_names, -# ) -# feature_service_keys = feature_service_online_features_dict.keys() -# expected_feature_refs = [ -# f"{projection.name_to_use()}__{feature.name}" -# if full_feature_names -# else feature.name -# for projection in feature_service.feature_view_projections -# for feature in projection.features -# ] -# assert set(feature_service_keys) == set(expected_feature_refs) | { -# "customer_id", -# "driver_id", -# } + tc = unittest.TestCase() + for i, entity_row in enumerate(entity_rows): + df_features = get_latest_feature_values_from_dataframes( + driver_df=drivers_df, + customer_df=customers_df, + orders_df=orders_df, + global_df=global_df, + entity_row=entity_row, + ) + tc.assertAlmostEqual( + feature_service_online_features_dict[ + response_feature_name( + "conv_rate_plus_100", expected_feature_refs, full_feature_names + ) + ][i], + df_features["conv_rate"] + 100, + delta=0.0001, + ) -# tc = unittest.TestCase() -# for i, entity_row in enumerate(entity_rows): -# df_features = get_latest_feature_values_from_dataframes( -# driver_df=drivers_df, -# customer_df=customers_df, -# orders_df=orders_df, -# global_df=global_df, -# entity_row=entity_row, -# ) -# tc.assertAlmostEqual( -# feature_service_online_features_dict[ -# response_feature_name( -# "conv_rate_plus_100", expected_feature_refs, full_feature_names -# ) -# ][i], -# df_features["conv_rate"] + 100, -# delta=0.0001, -# ) +def assert_feature_service_entity_mapping_correctness( + environment, + endpoint, + feature_service, + entity_rows, + full_feature_names, + origins_df, + destinations_df, +): + if full_feature_names: + feature_service_online_features_dict = get_online_features_dict( + environment=environment, + endpoint=endpoint, + features=feature_service, + entity_rows=entity_rows, + full_feature_names=full_feature_names, + ) + feature_service_keys = feature_service_online_features_dict.keys() + + expected_features = [ + f"{projection.name_to_use()}__{feature.name}" + if full_feature_names + else feature.name + for projection in feature_service.feature_view_projections + for feature in projection.features + ] + assert set(feature_service_keys) == set(expected_features) | { + "destination_id", + "origin_id", + } -# def assert_feature_service_entity_mapping_correctness( -# environment, -# endpoint, -# feature_service, -# entity_rows, -# full_feature_names, -# origins_df, -# destinations_df, -# ): -# if full_feature_names: -# feature_service_online_features_dict = get_online_features_dict( -# environment=environment, -# endpoint=endpoint, -# features=feature_service, -# entity_rows=entity_rows, -# full_feature_names=full_feature_names, -# ) -# feature_service_keys = feature_service_online_features_dict.keys() - -# expected_features = [ -# f"{projection.name_to_use()}__{feature.name}" -# if full_feature_names -# else feature.name -# for projection in feature_service.feature_view_projections -# for feature in projection.features -# ] -# assert set(feature_service_keys) == set(expected_features) | { -# "destination_id", -# "origin_id", -# } - -# for i, entity_row in enumerate(entity_rows): -# df_features = get_latest_feature_values_for_location_df( -# origin_df=origins_df, -# destination_df=destinations_df, -# entity_row=entity_row, -# ) -# for feature_name in ["origin__temperature", "destination__temperature"]: -# assert ( -# feature_service_online_features_dict[feature_name][i] -# == df_features[feature_name] -# ) -# else: -# # using 2 of the same FeatureView without full_feature_names=True will result in collision -# with pytest.raises(FeatureNameCollisionError): -# get_online_features_dict( -# environment=environment, -# endpoint=endpoint, -# features=feature_service, -# entity_rows=entity_rows, -# full_feature_names=full_feature_names, -# ) + for i, entity_row in enumerate(entity_rows): + df_features = get_latest_feature_values_for_location_df( + origin_df=origins_df, + destination_df=destinations_df, + entity_row=entity_row, + ) + for feature_name in ["origin__temperature", "destination__temperature"]: + assert ( + feature_service_online_features_dict[feature_name][i] + == df_features[feature_name] + ) + else: + # using 2 of the same FeatureView without full_feature_names=True will result in collision + with pytest.raises(FeatureNameCollisionError): + get_online_features_dict( + environment=environment, + endpoint=endpoint, + features=feature_service, + entity_rows=entity_rows, + full_feature_names=full_feature_names, + ) From d0c679ce369ee58f1d50903a013484ee0e1db24a Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 21 Jun 2022 15:08:14 -0700 Subject: [PATCH 21/26] Fix test Signed-off-by: Kevin Zhang --- .../test_stream_feature_view_apply.py | 138 +++++++++--------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/sdk/python/tests/integration/registration/test_stream_feature_view_apply.py b/sdk/python/tests/integration/registration/test_stream_feature_view_apply.py index 8e2af031c5f..f92fd340f09 100644 --- a/sdk/python/tests/integration/registration/test_stream_feature_view_apply.py +++ b/sdk/python/tests/integration/registration/test_stream_feature_view_apply.py @@ -77,72 +77,72 @@ def simple_sfv(df): assert features["dummy_field"] == [None] -@pytest.mark.integration -def test_stream_feature_view_udf(simple_dataset_1) -> None: - """ - Test apply of StreamFeatureView udfs are serialized correctly and usable. - """ - runner = CliRunner() - with runner.local_repo( - get_example_repo("example_feature_repo_1.py"), "bigquery" - ) as fs, prep_file_source( - df=simple_dataset_1, timestamp_field="ts_1" - ) as file_source: - entity = Entity(name="driver_entity", join_keys=["test_key"]) - - stream_source = KafkaSource( - name="kafka", - timestamp_field="event_timestamp", - kafka_bootstrap_servers="", - message_format=AvroFormat(""), - topic="topic", - batch_source=file_source, - watermark_delay_threshold=timedelta(days=1), - ) - - @stream_feature_view( - entities=[entity], - ttl=timedelta(days=30), - owner="test@example.com", - online=True, - schema=[Field(name="dummy_field", dtype=Float32)], - description="desc", - aggregations=[ - Aggregation( - column="dummy_field", function="max", time_window=timedelta(days=1), - ), - Aggregation( - column="dummy_field2", - function="count", - time_window=timedelta(days=24), - ), - ], - timestamp_field="event_timestamp", - mode="spark", - source=stream_source, - tags={}, - ) - def pandas_view(pandas_df): - import pandas as pd - - assert type(pandas_df) == pd.DataFrame - df = pandas_df.transform(lambda x: x + 10, axis=1) - df.insert(2, "C", [20.2, 230.0, 34.0], True) - return df - - import pandas as pd - - fs.apply([entity, pandas_view]) - - stream_feature_views = fs.list_stream_feature_views() - assert len(stream_feature_views) == 1 - assert stream_feature_views[0] == pandas_view - - sfv = stream_feature_views[0] - - df = pd.DataFrame({"A": [1, 2, 3], "B": [10, 20, 30]}) - new_df = sfv.udf(df) - expected_df = pd.DataFrame( - {"A": [11, 12, 13], "B": [20, 30, 40], "C": [20.2, 230.0, 34.0]} - ) - assert new_df.equals(expected_df) +# @pytest.mark.integration +# def test_stream_feature_view_udf(simple_dataset_1) -> None: +# """ +# Test apply of StreamFeatureView udfs are serialized correctly and usable. +# """ +# runner = CliRunner() +# with runner.local_repo( +# get_example_repo("example_feature_repo_1.py"), "bigquery" +# ) as fs, prep_file_source( +# df=simple_dataset_1, timestamp_field="ts_1" +# ) as file_source: +# entity = Entity(name="driver_entity", join_keys=["test_key"]) + +# stream_source = KafkaSource( +# name="kafka", +# timestamp_field="event_timestamp", +# kafka_bootstrap_servers="", +# message_format=AvroFormat(""), +# topic="topic", +# batch_source=file_source, +# watermark_delay_threshold=timedelta(days=1), +# ) + +# @stream_feature_view( +# entities=[entity], +# ttl=timedelta(days=30), +# owner="test@example.com", +# online=True, +# schema=[Field(name="dummy_field", dtype=Float32)], +# description="desc", +# aggregations=[ +# Aggregation( +# column="dummy_field", function="max", time_window=timedelta(days=1), +# ), +# Aggregation( +# column="dummy_field2", +# function="count", +# time_window=timedelta(days=24), +# ), +# ], +# timestamp_field="event_timestamp", +# mode="spark", +# source=stream_source, +# tags={}, +# ) +# def pandas_view(pandas_df): +# import pandas as pd + +# assert type(pandas_df) == pd.DataFrame +# df = pandas_df.transform(lambda x: x + 10, axis=1) +# df.insert(2, "C", [20.2, 230.0, 34.0], True) +# return df + +# import pandas as pd + +# fs.apply([entity, pandas_view]) + +# stream_feature_views = fs.list_stream_feature_views() +# assert len(stream_feature_views) == 1 +# assert stream_feature_views[0] == pandas_view + +# sfv = stream_feature_views[0] + +# df = pd.DataFrame({"A": [1, 2, 3], "B": [10, 20, 30]}) +# new_df = sfv.udf(df) +# expected_df = pd.DataFrame( +# {"A": [11, 12, 13], "B": [20, 30, 40], "C": [20.2, 230.0, 34.0]} +# ) +# assert new_df.equals(expected_df) From eea45c9220f7c0ad4a6087eaa2e96146417fe2f8 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 21 Jun 2022 15:51:27 -0700 Subject: [PATCH 22/26] add back in Signed-off-by: Kevin Zhang --- .../online_store/test_universal_online.py | 148 +++++++++--------- .../test_stream_feature_view_apply.py | 138 ++++++++-------- 2 files changed, 143 insertions(+), 143 deletions(-) diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index 9d4db3e03e5..c068e041116 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -441,80 +441,80 @@ def test_online_retrieval_with_event_timestamps( ) -# @pytest.mark.integration -# @pytest.mark.universal_online_stores -# # @pytest.mark.goserver Disabling because the go fs tests are flaking in CI. TODO(achals): uncomment after fixed. -# @pytest.mark.parametrize("full_feature_names", [True, False], ids=lambda v: str(v)) -# def test_stream_feature_view_online_retrieval( -# environment, universal_data_sources, feature_server_endpoint, full_feature_names -# ): -# """ -# Tests materialization and online retrieval for stream feature views. - -# This test is separate from test_online_retrieval since combining feature views and -# stream feature views into a single test resulted in test flakiness. This is tech -# debt that should be resolved soon. -# """ -# # Set up feature store. -# fs = environment.feature_store -# entities, datasets, data_sources = universal_data_sources -# feature_views = construct_universal_feature_views(data_sources) -# pushable_feature_view = feature_views.pushed_locations -# fs.apply([location(), pushable_feature_view]) - -# # Materialize. -# fs.materialize( -# environment.start_date - timedelta(days=1), -# environment.end_date + timedelta(days=1), -# ) - -# # Get online features by randomly sampling 10 entities that exist in the batch source. -# sample_locations = datasets.location_df.sample(10)["location_id"] -# entity_rows = [ -# {"location_id": sample_location} for sample_location in sample_locations -# ] - -# feature_refs = [ -# "pushable_location_stats:temperature", -# ] -# unprefixed_feature_refs = [f.rsplit(":", 1)[-1] for f in feature_refs if ":" in f] - -# online_features_dict = get_online_features_dict( -# environment=environment, -# endpoint=feature_server_endpoint, -# features=feature_refs, -# entity_rows=entity_rows, -# full_feature_names=full_feature_names, -# ) - -# # Check that the response has the expected set of keys. -# keys = set(online_features_dict.keys()) -# expected_keys = set( -# f.replace(":", "__") if full_feature_names else f.split(":")[-1] -# for f in feature_refs -# ) | {"location_id"} -# assert ( -# keys == expected_keys -# ), f"Response keys are different from expected: {keys - expected_keys} (extra) and {expected_keys - keys} (missing)" - -# # Check that the feature values match. -# tc = unittest.TestCase() -# for i, entity_row in enumerate(entity_rows): -# df_features = get_latest_feature_values_from_location_df( -# entity_row, datasets.location_df -# ) - -# assert df_features["location_id"] == online_features_dict["location_id"][i] -# for unprefixed_feature_ref in unprefixed_feature_refs: -# tc.assertAlmostEqual( -# df_features[unprefixed_feature_ref], -# online_features_dict[ -# response_feature_name( -# unprefixed_feature_ref, feature_refs, full_feature_names -# ) -# ][i], -# delta=0.0001, -# ) +@pytest.mark.integration +@pytest.mark.universal_online_stores +# @pytest.mark.goserver Disabling because the go fs tests are flaking in CI. TODO(achals): uncomment after fixed. +@pytest.mark.parametrize("full_feature_names", [True, False], ids=lambda v: str(v)) +def test_stream_feature_view_online_retrieval( + environment, universal_data_sources, feature_server_endpoint, full_feature_names +): + """ + Tests materialization and online retrieval for stream feature views. + + This test is separate from test_online_retrieval since combining feature views and + stream feature views into a single test resulted in test flakiness. This is tech + debt that should be resolved soon. + """ + # Set up feature store. + fs = environment.feature_store + entities, datasets, data_sources = universal_data_sources + feature_views = construct_universal_feature_views(data_sources) + pushable_feature_view = feature_views.pushed_locations + fs.apply([location(), pushable_feature_view]) + + # Materialize. + fs.materialize( + environment.start_date - timedelta(days=1), + environment.end_date + timedelta(days=1), + ) + + # Get online features by randomly sampling 10 entities that exist in the batch source. + sample_locations = datasets.location_df.sample(10)["location_id"] + entity_rows = [ + {"location_id": sample_location} for sample_location in sample_locations + ] + + feature_refs = [ + "pushable_location_stats:temperature", + ] + unprefixed_feature_refs = [f.rsplit(":", 1)[-1] for f in feature_refs if ":" in f] + + online_features_dict = get_online_features_dict( + environment=environment, + endpoint=feature_server_endpoint, + features=feature_refs, + entity_rows=entity_rows, + full_feature_names=full_feature_names, + ) + + # Check that the response has the expected set of keys. + keys = set(online_features_dict.keys()) + expected_keys = set( + f.replace(":", "__") if full_feature_names else f.split(":")[-1] + for f in feature_refs + ) | {"location_id"} + assert ( + keys == expected_keys + ), f"Response keys are different from expected: {keys - expected_keys} (extra) and {expected_keys - keys} (missing)" + + # Check that the feature values match. + tc = unittest.TestCase() + for i, entity_row in enumerate(entity_rows): + df_features = get_latest_feature_values_from_location_df( + entity_row, datasets.location_df + ) + + assert df_features["location_id"] == online_features_dict["location_id"][i] + for unprefixed_feature_ref in unprefixed_feature_refs: + tc.assertAlmostEqual( + df_features[unprefixed_feature_ref], + online_features_dict[ + response_feature_name( + unprefixed_feature_ref, feature_refs, full_feature_names + ) + ][i], + delta=0.0001, + ) @pytest.mark.integration diff --git a/sdk/python/tests/integration/registration/test_stream_feature_view_apply.py b/sdk/python/tests/integration/registration/test_stream_feature_view_apply.py index f92fd340f09..8e2af031c5f 100644 --- a/sdk/python/tests/integration/registration/test_stream_feature_view_apply.py +++ b/sdk/python/tests/integration/registration/test_stream_feature_view_apply.py @@ -77,72 +77,72 @@ def simple_sfv(df): assert features["dummy_field"] == [None] -# @pytest.mark.integration -# def test_stream_feature_view_udf(simple_dataset_1) -> None: -# """ -# Test apply of StreamFeatureView udfs are serialized correctly and usable. -# """ -# runner = CliRunner() -# with runner.local_repo( -# get_example_repo("example_feature_repo_1.py"), "bigquery" -# ) as fs, prep_file_source( -# df=simple_dataset_1, timestamp_field="ts_1" -# ) as file_source: -# entity = Entity(name="driver_entity", join_keys=["test_key"]) - -# stream_source = KafkaSource( -# name="kafka", -# timestamp_field="event_timestamp", -# kafka_bootstrap_servers="", -# message_format=AvroFormat(""), -# topic="topic", -# batch_source=file_source, -# watermark_delay_threshold=timedelta(days=1), -# ) - -# @stream_feature_view( -# entities=[entity], -# ttl=timedelta(days=30), -# owner="test@example.com", -# online=True, -# schema=[Field(name="dummy_field", dtype=Float32)], -# description="desc", -# aggregations=[ -# Aggregation( -# column="dummy_field", function="max", time_window=timedelta(days=1), -# ), -# Aggregation( -# column="dummy_field2", -# function="count", -# time_window=timedelta(days=24), -# ), -# ], -# timestamp_field="event_timestamp", -# mode="spark", -# source=stream_source, -# tags={}, -# ) -# def pandas_view(pandas_df): -# import pandas as pd - -# assert type(pandas_df) == pd.DataFrame -# df = pandas_df.transform(lambda x: x + 10, axis=1) -# df.insert(2, "C", [20.2, 230.0, 34.0], True) -# return df - -# import pandas as pd - -# fs.apply([entity, pandas_view]) - -# stream_feature_views = fs.list_stream_feature_views() -# assert len(stream_feature_views) == 1 -# assert stream_feature_views[0] == pandas_view - -# sfv = stream_feature_views[0] - -# df = pd.DataFrame({"A": [1, 2, 3], "B": [10, 20, 30]}) -# new_df = sfv.udf(df) -# expected_df = pd.DataFrame( -# {"A": [11, 12, 13], "B": [20, 30, 40], "C": [20.2, 230.0, 34.0]} -# ) -# assert new_df.equals(expected_df) +@pytest.mark.integration +def test_stream_feature_view_udf(simple_dataset_1) -> None: + """ + Test apply of StreamFeatureView udfs are serialized correctly and usable. + """ + runner = CliRunner() + with runner.local_repo( + get_example_repo("example_feature_repo_1.py"), "bigquery" + ) as fs, prep_file_source( + df=simple_dataset_1, timestamp_field="ts_1" + ) as file_source: + entity = Entity(name="driver_entity", join_keys=["test_key"]) + + stream_source = KafkaSource( + name="kafka", + timestamp_field="event_timestamp", + kafka_bootstrap_servers="", + message_format=AvroFormat(""), + topic="topic", + batch_source=file_source, + watermark_delay_threshold=timedelta(days=1), + ) + + @stream_feature_view( + entities=[entity], + ttl=timedelta(days=30), + owner="test@example.com", + online=True, + schema=[Field(name="dummy_field", dtype=Float32)], + description="desc", + aggregations=[ + Aggregation( + column="dummy_field", function="max", time_window=timedelta(days=1), + ), + Aggregation( + column="dummy_field2", + function="count", + time_window=timedelta(days=24), + ), + ], + timestamp_field="event_timestamp", + mode="spark", + source=stream_source, + tags={}, + ) + def pandas_view(pandas_df): + import pandas as pd + + assert type(pandas_df) == pd.DataFrame + df = pandas_df.transform(lambda x: x + 10, axis=1) + df.insert(2, "C", [20.2, 230.0, 34.0], True) + return df + + import pandas as pd + + fs.apply([entity, pandas_view]) + + stream_feature_views = fs.list_stream_feature_views() + assert len(stream_feature_views) == 1 + assert stream_feature_views[0] == pandas_view + + sfv = stream_feature_views[0] + + df = pd.DataFrame({"A": [1, 2, 3], "B": [10, 20, 30]}) + new_df = sfv.udf(df) + expected_df = pd.DataFrame( + {"A": [11, 12, 13], "B": [20, 30, 40], "C": [20.2, 230.0, 34.0]} + ) + assert new_df.equals(expected_df) From d094834352878225ce267b4fcb2e30bb07898f2c Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Wed, 22 Jun 2022 10:52:48 -0700 Subject: [PATCH 23/26] Fix Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/passthrough_provider.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/feast/infra/passthrough_provider.py b/sdk/python/feast/infra/passthrough_provider.py index 9d18e6b249f..e702661641a 100644 --- a/sdk/python/feast/infra/passthrough_provider.py +++ b/sdk/python/feast/infra/passthrough_provider.py @@ -110,7 +110,7 @@ def offline_write_batch( set_usage_attribute("provider", self.__class__.__name__) if self.offline_store: - self.offline_store.offline_write_batch(config, feature_view, data, progress) + self.offline_store.__class__.offline_write_batch(config, feature_view, data, progress) @log_exceptions_and_usage(sampler=RatioSampler(ratio=0.001)) def online_read( From 9f53598335c044c914855c5388face4548ed5db4 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Wed, 22 Jun 2022 10:53:26 -0700 Subject: [PATCH 24/26] lint Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/passthrough_provider.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/infra/passthrough_provider.py b/sdk/python/feast/infra/passthrough_provider.py index e702661641a..8c6dd831dde 100644 --- a/sdk/python/feast/infra/passthrough_provider.py +++ b/sdk/python/feast/infra/passthrough_provider.py @@ -110,7 +110,9 @@ def offline_write_batch( set_usage_attribute("provider", self.__class__.__name__) if self.offline_store: - self.offline_store.__class__.offline_write_batch(config, feature_view, data, progress) + self.offline_store.__class__.offline_write_batch( + config, feature_view, data, progress + ) @log_exceptions_and_usage(sampler=RatioSampler(ratio=0.001)) def online_read( From 80df2663f18425607bb36409e12f89268bb16b76 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Wed, 22 Jun 2022 11:30:47 -0700 Subject: [PATCH 25/26] Address review comments Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/passthrough_provider.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/feast/infra/passthrough_provider.py b/sdk/python/feast/infra/passthrough_provider.py index 8c6dd831dde..4c6a3c220da 100644 --- a/sdk/python/feast/infra/passthrough_provider.py +++ b/sdk/python/feast/infra/passthrough_provider.py @@ -110,7 +110,7 @@ def offline_write_batch( set_usage_attribute("provider", self.__class__.__name__) if self.offline_store: - self.offline_store.__class__.offline_write_batch( + self.offline_store.offline_write_batch( config, feature_view, data, progress ) From aa4794daafbe0fa5fddd847e19e8ff2e11a99136 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Wed, 22 Jun 2022 11:31:32 -0700 Subject: [PATCH 26/26] Fix Signed-off-by: Kevin Zhang --- sdk/python/feast/infra/passthrough_provider.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sdk/python/feast/infra/passthrough_provider.py b/sdk/python/feast/infra/passthrough_provider.py index 4c6a3c220da..9d18e6b249f 100644 --- a/sdk/python/feast/infra/passthrough_provider.py +++ b/sdk/python/feast/infra/passthrough_provider.py @@ -110,9 +110,7 @@ def offline_write_batch( set_usage_attribute("provider", self.__class__.__name__) if self.offline_store: - self.offline_store.offline_write_batch( - config, feature_view, data, progress - ) + self.offline_store.offline_write_batch(config, feature_view, data, progress) @log_exceptions_and_usage(sampler=RatioSampler(ratio=0.001)) def online_read(