diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index 6312c42a053..ce656f8e24f 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -60,3 +60,10 @@ def __init__(self, extras_type: str, nested_error: str): + f"You may need run {Style.BRIGHT + Fore.GREEN}pip install 'feast[{extras_type}]'{Style.RESET_ALL}" ) super().__init__(message) + + +class FeastOfflineStoreUnsupportedDataSource(Exception): + def __init__(self, offline_store_name: str, data_source_name: str): + super().__init__( + f"Offline Store '{offline_store_name}' does not support data source '{data_source_name}'" + ) diff --git a/sdk/python/feast/infra/gcp.py b/sdk/python/feast/infra/gcp.py index 627255cdac5..aa5284c441b 100644 --- a/sdk/python/feast/infra/gcp.py +++ b/sdk/python/feast/infra/gcp.py @@ -5,7 +5,6 @@ import mmh3 import pandas -import pyarrow from tqdm import tqdm from feast import FeatureTable, utils @@ -13,7 +12,7 @@ from feast.errors import FeastProviderLoginError from feast.feature_view import FeatureView from feast.infra.key_encoding_utils import serialize_entity_key -from feast.infra.offline_stores.helpers import get_offline_store_from_sources +from feast.infra.offline_stores.helpers import get_offline_store_from_config from feast.infra.provider import ( Provider, RetrievalJob, @@ -28,7 +27,7 @@ try: from google.auth.exceptions import DefaultCredentialsError - from google.cloud import bigquery, datastore + from google.cloud import datastore except ImportError as e: from feast.errors import FeastExtrasDependencyImportError @@ -40,11 +39,14 @@ class GcpProvider(Provider): def __init__(self, config: RepoConfig): assert isinstance(config.online_store, DatastoreOnlineStoreConfig) + assert config.offline_store is not None if config and config.online_store and config.online_store.project_id: self._gcp_project_id = config.online_store.project_id else: self._gcp_project_id = None + self.offline_store = get_offline_store_from_config(config.offline_store) + def _initialize_client(self): try: if self._gcp_project_id is not None: @@ -168,8 +170,7 @@ def materialize_single_feature_view( start_date = utils.make_tzaware(start_date) end_date = utils.make_tzaware(end_date) - offline_store = get_offline_store_from_sources([feature_view.input]) - table = offline_store.pull_latest_from_table_or_query( + table = self.offline_store.pull_latest_from_table_or_query( data_source=feature_view.input, join_key_columns=join_key_columns, feature_name_columns=feature_name_columns, @@ -193,14 +194,8 @@ def materialize_single_feature_view( feature_view.materialization_intervals.append((start_date, end_date)) registry.apply_feature_view(feature_view, project) - @staticmethod - def _pull_query(query: str) -> pyarrow.Table: - client = bigquery.Client() - query_job = client.query(query) - return query_job.to_arrow() - - @staticmethod def get_historical_features( + self, config: RepoConfig, feature_views: List[FeatureView], feature_refs: List[str], @@ -208,10 +203,7 @@ def get_historical_features( registry: Registry, project: str, ) -> RetrievalJob: - offline_store = get_offline_store_from_sources( - [feature_view.input for feature_view in feature_views] - ) - job = offline_store.get_historical_features( + job = self.offline_store.get_historical_features( config=config, feature_views=feature_views, feature_refs=feature_refs, diff --git a/sdk/python/feast/infra/local.py b/sdk/python/feast/infra/local.py index 0d6b16c9509..9f4450b22b1 100644 --- a/sdk/python/feast/infra/local.py +++ b/sdk/python/feast/infra/local.py @@ -12,7 +12,7 @@ from feast.entity import Entity from feast.feature_view import FeatureView from feast.infra.key_encoding_utils import serialize_entity_key -from feast.infra.offline_stores.helpers import get_offline_store_from_sources +from feast.infra.offline_stores.helpers import get_offline_store_from_config from feast.infra.provider import ( Provider, RetrievalJob, @@ -30,16 +30,15 @@ class LocalProvider(Provider): _db_path: Path def __init__(self, config: RepoConfig, repo_path: Path): - assert config is not None - assert config.online_store is not None - local_online_store_config = config.online_store - assert isinstance(local_online_store_config, SqliteOnlineStoreConfig) - local_path = Path(local_online_store_config.path) + assert isinstance(config.online_store, SqliteOnlineStoreConfig) + assert config.offline_store is not None + local_path = Path(config.online_store.path) if local_path.is_absolute(): self._db_path = local_path else: self._db_path = repo_path.joinpath(local_path) + self.offline_store = get_offline_store_from_config(config.offline_store) def _get_conn(self): Path(self._db_path).parent.mkdir(exist_ok=True) @@ -184,8 +183,7 @@ def materialize_single_feature_view( start_date = utils.make_tzaware(start_date) end_date = utils.make_tzaware(end_date) - offline_store = get_offline_store_from_sources([feature_view.input]) - table = offline_store.pull_latest_from_table_or_query( + table = self.offline_store.pull_latest_from_table_or_query( data_source=feature_view.input, join_key_columns=join_key_columns, feature_name_columns=feature_name_columns, @@ -209,8 +207,8 @@ def materialize_single_feature_view( feature_view.materialization_intervals.append((start_date, end_date)) registry.apply_feature_view(feature_view, project) - @staticmethod def get_historical_features( + self, config: RepoConfig, feature_views: List[FeatureView], feature_refs: List[str], @@ -218,10 +216,7 @@ def get_historical_features( registry: Registry, project: str, ) -> RetrievalJob: - offline_store = get_offline_store_from_sources( - [feature_view.input for feature_view in feature_views] - ) - return offline_store.get_historical_features( + return self.offline_store.get_historical_features( config=config, feature_views=feature_views, feature_refs=feature_refs, diff --git a/sdk/python/feast/infra/offline_stores/bigquery.py b/sdk/python/feast/infra/offline_stores/bigquery.py index 516f6fdc8f6..dd991f864b3 100644 --- a/sdk/python/feast/infra/offline_stores/bigquery.py +++ b/sdk/python/feast/infra/offline_stores/bigquery.py @@ -17,7 +17,7 @@ _get_requested_feature_views_to_features_dict, ) from feast.registry import Registry -from feast.repo_config import RepoConfig +from feast.repo_config import BigQueryOfflineStoreConfig, RepoConfig try: from google.auth.exceptions import DefaultCredentialsError @@ -100,8 +100,10 @@ def get_historical_features( entity_df ) + assert isinstance(config.offline_store, BigQueryOfflineStoreConfig) + table_id = _upload_entity_df_into_bigquery( - config.project, entity_df, client + config.project, config.offline_store.dataset, entity_df, client, ) entity_df_sql_table = f"`{table_id}`" else: @@ -198,11 +200,11 @@ class FeatureViewQueryContext: entity_selections: List[str] -def _upload_entity_df_into_bigquery(project, entity_df, client) -> str: +def _upload_entity_df_into_bigquery(project, dataset_name, entity_df, client) -> str: """Uploads a Pandas entity dataframe into a BigQuery table and returns a reference to the resulting table""" # First create the BigQuery dataset if it doesn't exist - dataset = bigquery.Dataset(f"{client.project}.feast_{project}") + dataset = bigquery.Dataset(f"{client.project}.{dataset_name}") dataset.location = "US" client.create_dataset( dataset, exists_ok=True @@ -213,7 +215,7 @@ def _upload_entity_df_into_bigquery(project, entity_df, client) -> str: # Upload the dataframe into BigQuery, creating a temporary table job_config = bigquery.LoadJobConfig() - table_id = f"{client.project}.feast_{project}.entity_df_{int(time.time())}" + table_id = f"{client.project}.{dataset_name}.entity_df_{project}_{int(time.time())}" job = client.load_table_from_dataframe(entity_df, table_id, job_config=job_config,) job.result() diff --git a/sdk/python/feast/infra/offline_stores/helpers.py b/sdk/python/feast/infra/offline_stores/helpers.py index b5860df5364..af1d1b92123 100644 --- a/sdk/python/feast/infra/offline_stores/helpers.py +++ b/sdk/python/feast/infra/offline_stores/helpers.py @@ -1,28 +1,41 @@ -from typing import List - from feast.data_source import BigQuerySource, DataSource, FileSource +from feast.errors import FeastOfflineStoreUnsupportedDataSource from feast.infra.offline_stores.offline_store import OfflineStore +from feast.repo_config import ( + BigQueryOfflineStoreConfig, + FileOfflineStoreConfig, + OfflineStoreConfig, +) -def get_offline_store_from_sources(sources: List[DataSource]) -> OfflineStore: - """Detect which offline store should be used for retrieving historical features""" - - source_types = [type(source) for source in sources] +def get_offline_store_from_config( + offline_store_config: OfflineStoreConfig, +) -> OfflineStore: + """Get the offline store from offline store config""" - # Retrieve features from ParquetOfflineStore - if all(source == FileSource for source in source_types): + if isinstance(offline_store_config, FileOfflineStoreConfig): from feast.infra.offline_stores.file import FileOfflineStore return FileOfflineStore() - - # Retrieve features from BigQueryOfflineStore - if all(source == BigQuerySource for source in source_types): + elif isinstance(offline_store_config, BigQueryOfflineStoreConfig): from feast.infra.offline_stores.bigquery import BigQueryOfflineStore return BigQueryOfflineStore() - # Could not map inputs to an OfflineStore implementation - raise NotImplementedError( - "Unsupported combination of feature view input source types. Please ensure that all source types are " - "consistent and available in the same offline store." + raise ValueError(f"Unsupported offline store config '{offline_store_config}'") + + +def assert_offline_store_supports_data_source( + offline_store_config: OfflineStoreConfig, data_source: DataSource +): + if ( + isinstance(offline_store_config, FileOfflineStoreConfig) + and isinstance(data_source, FileSource) + ) or ( + isinstance(offline_store_config, BigQueryOfflineStoreConfig) + and isinstance(data_source, BigQuerySource) + ): + return + raise FeastOfflineStoreUnsupportedDataSource( + offline_store_config.type, data_source.__class__.__name__ ) diff --git a/sdk/python/feast/infra/provider.py b/sdk/python/feast/infra/provider.py index 04c1bfce9e3..05dac141c85 100644 --- a/sdk/python/feast/infra/provider.py +++ b/sdk/python/feast/infra/provider.py @@ -107,9 +107,9 @@ def materialize_single_feature_view( ) -> None: pass - @staticmethod @abc.abstractmethod def get_historical_features( + self, config: RepoConfig, feature_views: List[FeatureView], feature_refs: List[str], diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index bb18119b427..90afc7b80ef 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -37,6 +37,26 @@ class DatastoreOnlineStoreConfig(FeastBaseModel): OnlineStoreConfig = Union[DatastoreOnlineStoreConfig, SqliteOnlineStoreConfig] +class FileOfflineStoreConfig(FeastBaseModel): + """ Offline store config for local (file-based) store """ + + type: Literal["file"] = "file" + """ Offline store type selector""" + + +class BigQueryOfflineStoreConfig(FeastBaseModel): + """ Offline store config for GCP BigQuery """ + + type: Literal["bigquery"] = "bigquery" + """ Offline store type selector""" + + dataset: Optional[StrictStr] = "feast" + """ (optional) BigQuery Dataset name for temporary tables """ + + +OfflineStoreConfig = Union[FileOfflineStoreConfig, BigQueryOfflineStoreConfig] + + class RegistryConfig(FeastBaseModel): """ Metadata Store Configuration. Configuration that relates to reading from and writing to the Feast registry.""" @@ -68,6 +88,9 @@ class RepoConfig(FeastBaseModel): online_store: OnlineStoreConfig = SqliteOnlineStoreConfig() """ OnlineStoreConfig: Online store configuration (optional depending on provider) """ + offline_store: OfflineStoreConfig = FileOfflineStoreConfig() + """ OfflineStoreConfig: Offline store configuration (optional depending on provider) """ + def get_registry_config(self): if isinstance(self.registry, str): return RegistryConfig(path=self.registry) @@ -82,45 +105,84 @@ def _validate_online_store_config(cls, values): # considered tech debt until we can implement https://github.com/samuelcolvin/pydantic/issues/619 or a more # granular configuration system - # Skip if online store isn't set explicitly + # Set empty online_store config if it isn't set explicitly if "online_store" not in values: values["online_store"] = dict() - # Skip if we arent creating the configuration from a dict + # Skip if we aren't creating the configuration from a dict if not isinstance(values["online_store"], Dict): return values # Make sure that the provider configuration is set. We need it to set the defaults assert "provider" in values - if "online_store" in values: - # Set the default type - if "type" not in values["online_store"]: - if values["provider"] == "local": - values["online_store"]["type"] = "sqlite" - elif values["provider"] == "gcp": - values["online_store"]["type"] = "datastore" - - online_store_type = values["online_store"]["type"] - - # Make sure the user hasn't provided the wrong type - assert online_store_type in ["datastore", "sqlite"] - - # Validate the dict to ensure one of the union types match - try: - if online_store_type == "sqlite": - SqliteOnlineStoreConfig(**values["online_store"]) - elif values["online_store"]["type"] == "datastore": - DatastoreOnlineStoreConfig(**values["online_store"]) - else: - raise ValidationError( - f"Invalid online store type {online_store_type}" - ) - except ValidationError as e: + # Set the default type + if "type" not in values["online_store"]: + if values["provider"] == "local": + values["online_store"]["type"] = "sqlite" + elif values["provider"] == "gcp": + values["online_store"]["type"] = "datastore" + + online_store_type = values["online_store"]["type"] + + # Make sure the user hasn't provided the wrong type + assert online_store_type in ["datastore", "sqlite"] + + # Validate the dict to ensure one of the union types match + try: + if online_store_type == "sqlite": + SqliteOnlineStoreConfig(**values["online_store"]) + elif online_store_type == "datastore": + DatastoreOnlineStoreConfig(**values["online_store"]) + else: + raise ValidationError(f"Invalid online store type {online_store_type}") + except ValidationError as e: + raise ValidationError( + [ErrorWrapper(e, loc="online_store")], model=SqliteOnlineStoreConfig, + ) + + return values + + @root_validator(pre=True) + def _validate_offline_store_config(cls, values): + # Set empty offline_store config if it isn't set explicitly + if "offline_store" not in values: + values["offline_store"] = dict() + + # Skip if we aren't creating the configuration from a dict + if not isinstance(values["offline_store"], Dict): + return values + + # Make sure that the provider configuration is set. We need it to set the defaults + assert "provider" in values + + # Set the default type + if "type" not in values["offline_store"]: + if values["provider"] == "local": + values["offline_store"]["type"] = "file" + elif values["provider"] == "gcp": + values["offline_store"]["type"] = "bigquery" + + offline_store_type = values["offline_store"]["type"] + + # Make sure the user hasn't provided the wrong type + assert offline_store_type in ["file", "bigquery"] + + # Validate the dict to ensure one of the union types match + try: + if offline_store_type == "file": + FileOfflineStoreConfig(**values["offline_store"]) + elif offline_store_type == "bigquery": + BigQueryOfflineStoreConfig(**values["offline_store"]) + else: raise ValidationError( - [ErrorWrapper(e, loc="online_store")], - model=SqliteOnlineStoreConfig, + f"Invalid offline store type {offline_store_type}" ) + except ValidationError as e: + raise ValidationError( + [ErrorWrapper(e, loc="offline_store")], model=FileOfflineStoreConfig, + ) + return values diff --git a/sdk/python/feast/repo_operations.py b/sdk/python/feast/repo_operations.py index 552d3a4bc35..b9209faa4b3 100644 --- a/sdk/python/feast/repo_operations.py +++ b/sdk/python/feast/repo_operations.py @@ -11,6 +11,7 @@ from feast import Entity, FeatureTable from feast.feature_view import FeatureView +from feast.infra.offline_stores.helpers import assert_offline_store_supports_data_source from feast.infra.provider import get_provider from feast.names import adjectives, animals from feast.registry import Registry @@ -130,6 +131,14 @@ def apply_total(repo_config: RepoConfig, repo_path: Path): for t in repo.feature_views: repo_table_names.add(t.name) + data_sources = [t.input for t in repo.feature_views] + + # Make sure the data source used by this feature view is supported by + for data_source in data_sources: + assert_offline_store_supports_data_source( + repo_config.offline_store, data_source + ) + tables_to_delete = [] for registry_table in registry.list_feature_tables(project=project): if registry_table.name not in repo_table_names: diff --git a/sdk/python/tests/cli_utils.py b/sdk/python/tests/cli_utils.py index 11de6ace802..c7eb20301c8 100644 --- a/sdk/python/tests/cli_utils.py +++ b/sdk/python/tests/cli_utils.py @@ -40,7 +40,7 @@ def run_with_output(self, args: List[str], cwd: Path) -> Tuple[int, bytes]: return e.returncode, e.output @contextmanager - def local_repo(self, example_repo_py: str): + def local_repo(self, example_repo_py: str, offline_store: str): """ Convenience method to set up all the boilerplate for a local feature repo. """ @@ -63,6 +63,8 @@ def local_repo(self, example_repo_py: str): provider: local online_store: path: {data_path / "online_store.db"} + offline_store: + type: {offline_store} """ ) ) diff --git a/sdk/python/tests/test_cli_local.py b/sdk/python/tests/test_cli_local.py index 1ac20c16bb4..5b1a988a99b 100644 --- a/sdk/python/tests/test_cli_local.py +++ b/sdk/python/tests/test_cli_local.py @@ -31,6 +31,8 @@ def test_workflow() -> None: provider: local online_store: path: {data_path / "online_store.db"} + offline_store: + type: bigquery """ ) ) @@ -96,6 +98,8 @@ def test_non_local_feature_repo() -> None: provider: local online_store: path: data/online_store.db + offline_store: + type: bigquery """ ) ) @@ -133,6 +137,8 @@ def setup_third_party_provider_repo(provider_name: str): online_store: path: data/online_store.db type: sqlite + offline_store: + type: file """ ) ) diff --git a/sdk/python/tests/test_e2e_local.py b/sdk/python/tests/test_e2e_local.py index 596ff5783e0..0714f5e91b4 100644 --- a/sdk/python/tests/test_e2e_local.py +++ b/sdk/python/tests/test_e2e_local.py @@ -76,7 +76,8 @@ def test_e2e_local() -> None: with runner.local_repo( get_example_repo("example_feature_repo_2.py").replace( "%PARQUET_PATH%", driver_stats_path - ) + ), + "file", ) as store: assert store.repo_path is not None diff --git a/sdk/python/tests/test_historical_retrieval.py b/sdk/python/tests/test_historical_retrieval.py index ac2d091ca12..84d7777954e 100644 --- a/sdk/python/tests/test_historical_retrieval.py +++ b/sdk/python/tests/test_historical_retrieval.py @@ -5,6 +5,7 @@ from datetime import datetime, timedelta from tempfile import TemporaryDirectory +import assertpy import numpy as np import pandas as pd import pytest @@ -20,7 +21,11 @@ from feast.feature_store import FeatureStore from feast.feature_view import FeatureView from feast.infra.provider import DEFAULT_ENTITY_DF_EVENT_TIMESTAMP_COL -from feast.repo_config import RepoConfig, SqliteOnlineStoreConfig +from feast.repo_config import ( + BigQueryOfflineStoreConfig, + RepoConfig, + SqliteOnlineStoreConfig, +) from feast.value_type import ValueType np.random.seed(0) @@ -312,7 +317,7 @@ def test_historical_features_from_parquet_sources(infer_event_timestamp_col): @pytest.mark.integration @pytest.mark.parametrize( - "provider_type", ["local", "gcp"], + "provider_type", ["local", "gcp", "gcp_custom_offline_config"], ) @pytest.mark.parametrize( "infer_event_timestamp_col", [False, True], @@ -379,6 +384,7 @@ def test_historical_features_from_bigquery_sources( online_store=SqliteOnlineStoreConfig( path=os.path.join(temp_dir, "online_store.db"), ), + offline_store=BigQueryOfflineStoreConfig(type="bigquery",), ) ) elif provider_type == "gcp": @@ -389,6 +395,20 @@ def test_historical_features_from_bigquery_sources( random.choices(string.ascii_uppercase + string.digits, k=10) ), provider="gcp", + offline_store=BigQueryOfflineStoreConfig(type="bigquery",), + ) + ) + elif provider_type == "gcp_custom_offline_config": + store = FeatureStore( + config=RepoConfig( + registry=os.path.join(temp_dir, "registry.db"), + project="".join( + random.choices(string.ascii_uppercase + string.digits, k=10) + ), + provider="gcp", + offline_store=BigQueryOfflineStoreConfig( + type="bigquery", dataset="foo" + ), ) ) else: @@ -415,6 +435,7 @@ def test_historical_features_from_bigquery_sources( "customer_profile:lifetime_trip_count", ], ) + actual_df_from_sql_entities = job_from_sql.to_df() assert_frame_equal( @@ -437,6 +458,14 @@ def test_historical_features_from_bigquery_sources( "customer_profile:lifetime_trip_count", ], ) + + if provider_type == "gcp_custom_offline_config": + # Make sure that custom dataset name is being used from the offline_store config + assertpy.assert_that(job_from_df.query).contains("foo.entity_df") + else: + # If the custom dataset name isn't provided in the config, use default `feast` name + assertpy.assert_that(job_from_df.query).contains("feast.entity_df") + actual_df_from_df_entities = job_from_df.to_df() assert_frame_equal( diff --git a/sdk/python/tests/test_online_retrieval.py b/sdk/python/tests/test_online_retrieval.py index 89252687271..e68316f15b5 100644 --- a/sdk/python/tests/test_online_retrieval.py +++ b/sdk/python/tests/test_online_retrieval.py @@ -17,7 +17,9 @@ def test_online() -> None: Test reading from the online store in local mode. """ runner = CliRunner() - with runner.local_repo(get_example_repo("example_feature_repo_1.py")) as store: + with runner.local_repo( + get_example_repo("example_feature_repo_1.py"), "bigquery" + ) as store: # Write some data to two tables driver_locations_fv = store.get_feature_view(name="driver_locations") diff --git a/sdk/python/tests/test_partial_apply.py b/sdk/python/tests/test_partial_apply.py index a2c6adc688b..a6bf1915009 100644 --- a/sdk/python/tests/test_partial_apply.py +++ b/sdk/python/tests/test_partial_apply.py @@ -12,7 +12,9 @@ def test_partial() -> None: """ runner = CliRunner() - with runner.local_repo(get_example_repo("example_feature_repo_1.py")) as store: + with runner.local_repo( + get_example_repo("example_feature_repo_1.py"), "bigquery" + ) as store: driver_locations_source = BigQuerySource( table_ref="rh_prod.ride_hailing_co.drivers",