diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index a0b3e2bf49a..9f33dcd6857 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -42,15 +42,15 @@ def __init__(self, provider_name): super().__init__(f"Provider '{provider_name}' is not implemented") -class FeastProviderModuleImportError(Exception): - def __init__(self, module_name): - super().__init__(f"Could not import provider module '{module_name}'") +class FeastModuleImportError(Exception): + def __init__(self, module_name, module_type="provider"): + super().__init__(f"Could not import {module_type} module '{module_name}'") -class FeastProviderClassImportError(Exception): - def __init__(self, module_name, class_name): +class FeastClassImportError(Exception): + def __init__(self, module_name, class_name, class_type="provider"): super().__init__( - f"Could not import provider '{class_name}' from module '{module_name}'" + f"Could not import {class_type} '{class_name}' from module '{module_name}'" ) @@ -71,6 +71,20 @@ def __init__(self, offline_store_name: str, data_source_name: str): ) +class FeastOnlineStoreInvalidName(Exception): + def __init__(self, online_store_class_name: str): + super().__init__( + f"Online Store Class '{online_store_class_name}' should end with the string `OnlineStore`.'" + ) + + +class FeastOnlineStoreConfigInvalidName(Exception): + def __init__(self, online_store_config_class_name: str): + super().__init__( + f"Online Store Config Class '{online_store_config_class_name}' should end with the string `OnlineStoreConfig`.'" + ) + + class FeastOnlineStoreUnsupportedDataSource(Exception): def __init__(self, online_store_name: str, data_source_name: str): super().__init__( diff --git a/sdk/python/feast/infra/online_stores/datastore.py b/sdk/python/feast/infra/online_stores/datastore.py index 1f2c5abac49..c623af1c1f8 100644 --- a/sdk/python/feast/infra/online_stores/datastore.py +++ b/sdk/python/feast/infra/online_stores/datastore.py @@ -17,6 +17,8 @@ from typing import Any, Callable, Dict, Iterator, List, Optional, Sequence, Tuple, Union import mmh3 +from pydantic import PositiveInt, StrictStr +from pydantic.typing import Literal from feast import Entity, FeatureTable, utils from feast.feature_view import FeatureView @@ -24,7 +26,7 @@ from feast.infra.online_stores.online_store import OnlineStore from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto from feast.protos.feast.types.Value_pb2 import Value as ValueProto -from feast.repo_config import DatastoreOnlineStoreConfig, RepoConfig +from feast.repo_config import FeastConfigBaseModel, RepoConfig try: from google.auth.exceptions import DefaultCredentialsError @@ -40,6 +42,25 @@ ] +class DatastoreOnlineStoreConfig(FeastConfigBaseModel): + """ Online store config for GCP Datastore """ + + type: Literal["datastore"] = "datastore" + """ Online store type selector""" + + project_id: Optional[StrictStr] = None + """ (optional) GCP Project Id """ + + namespace: Optional[StrictStr] = None + """ (optional) Datastore namespace """ + + write_concurrency: Optional[PositiveInt] = 40 + """ (optional) Amount of threads to use when writing batches of feature rows into Datastore""" + + write_batch_size: Optional[PositiveInt] = 50 + """ (optional) Amount of feature rows per batch being written into Datastore""" + + class DatastoreOnlineStore(OnlineStore): """ OnlineStore is an object used for all interaction between Feast and the service used for offline storage of diff --git a/sdk/python/feast/infra/online_stores/helpers.py b/sdk/python/feast/infra/online_stores/helpers.py index 391794d20e0..71693ce1354 100644 --- a/sdk/python/feast/infra/online_stores/helpers.py +++ b/sdk/python/feast/infra/online_stores/helpers.py @@ -1,65 +1,41 @@ +import importlib import struct -from typing import Any, Dict, Set +from typing import Any import mmh3 -from feast.data_source import BigQuerySource, DataSource, FileSource -from feast.errors import FeastOnlineStoreUnsupportedDataSource +from feast import errors from feast.infra.online_stores.online_store import OnlineStore from feast.protos.feast.storage.Redis_pb2 import RedisKeyV2 as RedisKeyProto from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto -from feast.repo_config import ( - DatastoreOnlineStoreConfig, - OnlineStoreConfig, - RedisOnlineStoreConfig, - SqliteOnlineStoreConfig, -) -def get_online_store_from_config( - online_store_config: OnlineStoreConfig, -) -> OnlineStore: +def get_online_store_from_config(online_store_config: Any,) -> OnlineStore: """Get the offline store from offline store config""" - if isinstance(online_store_config, SqliteOnlineStoreConfig): - from feast.infra.online_stores.sqlite import SqliteOnlineStore - - return SqliteOnlineStore() - elif isinstance(online_store_config, DatastoreOnlineStoreConfig): - from feast.infra.online_stores.datastore import DatastoreOnlineStore - - return DatastoreOnlineStore() - elif isinstance(online_store_config, RedisOnlineStoreConfig): - from feast.infra.online_stores.redis import RedisOnlineStore - - return RedisOnlineStore() - raise ValueError(f"Unsupported offline store config '{online_store_config}'") - - -SUPPORTED_SOURCES: Dict[Any, Set[Any]] = { - SqliteOnlineStoreConfig: {FileSource}, - DatastoreOnlineStoreConfig: {BigQuerySource}, - RedisOnlineStoreConfig: {FileSource, BigQuerySource}, -} - - -def assert_online_store_supports_data_source( - online_store_config: OnlineStoreConfig, data_source: DataSource -): - supported_sources: Set[Any] = SUPPORTED_SOURCES.get( - online_store_config.__class__, set() - ) - # This is needed because checking for `in` with Union types breaks mypy. - # https://github.com/python/mypy/issues/4954 - # We can replace this with `data_source.__class__ in SUPPORTED_SOURCES[online_store_config.__class__]` - # Once ^ is resolved. - if supported_sources: - for source in supported_sources: - if source == data_source.__class__: - return - raise FeastOnlineStoreUnsupportedDataSource( - online_store_config.type, data_source.__class__.__name__ - ) + module_name = online_store_config.__module__ + qualified_name = type(online_store_config).__name__ + store_class_name = qualified_name.replace("Config", "") + try: + module = importlib.import_module(module_name) + except Exception as e: + # The original exception can be anything - either module not found, + # or any other kind of error happening during the module import time. + # So we should include the original error as well in the stack trace. + raise errors.FeastModuleImportError( + module_name, module_type="OnlineStore" + ) from e + + # Try getting the provider class definition + try: + online_store_class = getattr(module, store_class_name) + except AttributeError: + # This can only be one type of error, when class_name attribute does not exist in the module + # So we don't have to include the original exception here + raise errors.FeastClassImportError( + module_name, store_class_name, class_type="OnlineStore" + ) from None + return online_store_class() def _redis_key(project: str, entity_key: EntityKeyProto): diff --git a/sdk/python/feast/infra/online_stores/redis.py b/sdk/python/feast/infra/online_stores/redis.py index abab030dced..bb85a8e853d 100644 --- a/sdk/python/feast/infra/online_stores/redis.py +++ b/sdk/python/feast/infra/online_stores/redis.py @@ -13,16 +13,19 @@ # limitations under the License. import json from datetime import datetime +from enum import Enum from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple, Union from google.protobuf.timestamp_pb2 import Timestamp +from pydantic import StrictStr +from pydantic.typing import Literal from feast import Entity, FeatureTable, FeatureView, RepoConfig, utils from feast.infra.online_stores.helpers import _mmh3, _redis_key from feast.infra.online_stores.online_store import OnlineStore from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto from feast.protos.feast.types.Value_pb2 import Value as ValueProto -from feast.repo_config import RedisOnlineStoreConfig, RedisType +from feast.repo_config import FeastConfigBaseModel try: from redis import Redis @@ -35,6 +38,25 @@ EX_SECONDS = 253402300799 +class RedisType(str, Enum): + redis = "redis" + redis_cluster = "redis_cluster" + + +class RedisOnlineStoreConfig(FeastConfigBaseModel): + """Online store config for Redis store""" + + type: Literal["redis"] = "redis" + """Online store type selector""" + + redis_type: RedisType = RedisType.redis + """Redis type: redis or redis_cluster""" + + connection_string: StrictStr = "localhost:6379" + """Connection string containing the host, port, and configuration parameters for Redis + format: host:port,parameter1,parameter2 eg. redis:6379,db=0 """ + + class RedisOnlineStore(OnlineStore): _client: Optional[Union[Redis, RedisCluster]] = None @@ -99,7 +121,6 @@ def _get_client(self, online_store_config: RedisOnlineStoreConfig): startup_nodes, kwargs = self._parse_connection_string( online_store_config.connection_string ) - print(f"Startup nodes: {startup_nodes}, {kwargs}") if online_store_config.type == RedisType.redis_cluster: kwargs["startup_nodes"] = startup_nodes self._client = RedisCluster(**kwargs) diff --git a/sdk/python/feast/infra/online_stores/sqlite.py b/sdk/python/feast/infra/online_stores/sqlite.py index 9c2e5cd251a..7385ae25a94 100644 --- a/sdk/python/feast/infra/online_stores/sqlite.py +++ b/sdk/python/feast/infra/online_stores/sqlite.py @@ -19,6 +19,8 @@ from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple, Union import pytz +from pydantic import StrictStr +from pydantic.schema import Literal from feast import Entity, FeatureTable from feast.feature_view import FeatureView @@ -26,7 +28,17 @@ from feast.infra.online_stores.online_store import OnlineStore from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto from feast.protos.feast.types.Value_pb2 import Value as ValueProto -from feast.repo_config import RepoConfig +from feast.repo_config import FeastConfigBaseModel, RepoConfig + + +class SqliteOnlineStoreConfig(FeastConfigBaseModel): + """ Online store config for local (SQLite-based) store """ + + type: Literal["sqlite"] = "sqlite" + """ Online store type selector""" + + path: StrictStr = "data/online.db" + """ (optional) Path to sqlite db """ class SqliteOnlineStore(OnlineStore): @@ -65,6 +77,7 @@ def online_write_batch( ], progress: Optional[Callable[[int], Any]], ) -> None: + conn = self._get_conn(config) project = config.project diff --git a/sdk/python/feast/infra/provider.py b/sdk/python/feast/infra/provider.py index 5d4f8d6cf0c..9fe95132510 100644 --- a/sdk/python/feast/infra/provider.py +++ b/sdk/python/feast/infra/provider.py @@ -163,7 +163,7 @@ def get_provider(config: RepoConfig, repo_path: Path) -> Provider: # The original exception can be anything - either module not found, # or any other kind of error happening during the module import time. # So we should include the original error as well in the stack trace. - raise errors.FeastProviderModuleImportError(module_name) from e + raise errors.FeastModuleImportError(module_name) from e # Try getting the provider class definition try: @@ -171,9 +171,7 @@ def get_provider(config: RepoConfig, repo_path: Path) -> Provider: except AttributeError: # This can only be one type of error, when class_name attribute does not exist in the module # So we don't have to include the original exception here - raise errors.FeastProviderClassImportError( - module_name, class_name - ) from None + raise errors.FeastClassImportError(module_name, class_name) from None return ProviderCls(config, repo_path) diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index 7e70efb799b..25c55dcc2bc 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -1,80 +1,39 @@ -from enum import Enum +import importlib from pathlib import Path +from typing import Any import yaml -from pydantic import ( - BaseModel, - PositiveInt, - StrictInt, - StrictStr, - ValidationError, - root_validator, -) +from pydantic import BaseModel, StrictInt, StrictStr, ValidationError, root_validator from pydantic.error_wrappers import ErrorWrapper from pydantic.typing import Dict, Literal, Optional, Union +from feast import errors from feast.telemetry import log_exceptions +# This dict exists so that: +# - existing values for the online store type in featurestore.yaml files continue to work in a backwards compatible way +# - first party and third party implementations can use the same class loading code path. +ONLINE_CONFIG_CLASS_FOR_TYPE = { + "sqlite": "feast.infra.online_stores.sqlite.SqliteOnlineStore", + "datastore": "feast.infra.online_stores.datastore.DatastoreOnlineStore", + "redis": "feast.infra.online_stores.redis.RedisOnlineStore", +} + class FeastBaseModel(BaseModel): """ Feast Pydantic Configuration Class """ class Config: arbitrary_types_allowed = True - extra = "forbid" - - -class SqliteOnlineStoreConfig(FeastBaseModel): - """ Online store config for local (SQLite-based) store """ - - type: Literal["sqlite"] = "sqlite" - """ Online store type selector""" - - path: StrictStr = "data/online.db" - """ (optional) Path to sqlite db """ - - -class DatastoreOnlineStoreConfig(FeastBaseModel): - """ Online store config for GCP Datastore """ - - type: Literal["datastore"] = "datastore" - """ Online store type selector""" - - project_id: Optional[StrictStr] = None - """ (optional) GCP Project Id """ - - namespace: Optional[StrictStr] = None - """ (optional) Datastore namespace """ - - write_concurrency: Optional[PositiveInt] = 40 - """ (optional) Amount of threads to use when writing batches of feature rows into Datastore""" - - write_batch_size: Optional[PositiveInt] = 50 - """ (optional) Amount of feature rows per batch being written into Datastore""" - - -class RedisType(str, Enum): - redis = "redis" - redis_cluster = "redis_cluster" - + extra = "allow" -class RedisOnlineStoreConfig(FeastBaseModel): - """Online store config for Redis store""" - - type: Literal["redis"] = "redis" - """Online store type selector""" - - redis_type: RedisType = RedisType.redis - """Redis type: redis or redis_cluster""" - - connection_string: StrictStr = "localhost:6379" - """Connection string containing the host, port, and configuration parameters for Redis - format: host:port,parameter1,parameter2 eg. redis:6379,db=0 """ +class FeastConfigBaseModel(BaseModel): + """ Feast Pydantic Configuration Class """ -OnlineStoreConfig = Union[ - DatastoreOnlineStoreConfig, SqliteOnlineStoreConfig, RedisOnlineStoreConfig -] + class Config: + arbitrary_types_allowed = True + extra = "forbid" class FileOfflineStoreConfig(FeastBaseModel): @@ -123,9 +82,9 @@ class RepoConfig(FeastBaseModel): """ provider: StrictStr - """ str: local or gcp or redis """ + """ str: local or gcp """ - online_store: OnlineStoreConfig = SqliteOnlineStoreConfig() + online_store: Any """ OnlineStoreConfig: Online store configuration (optional depending on provider) """ offline_store: OfflineStoreConfig = FileOfflineStoreConfig() @@ -133,6 +92,13 @@ class RepoConfig(FeastBaseModel): repo_path: Optional[Path] = None + def __init__(self, **data: Any): + super().__init__(**data) + if isinstance(self.online_store, Dict): + self.online_store = get_online_config_from_type(self.online_store["type"])( + **self.online_store + ) + def get_registry_config(self): if isinstance(self.registry, str): return RegistryConfig(path=self.registry) @@ -160,6 +126,8 @@ def _validate_online_store_config(cls, values): assert "provider" in values # Set the default type + # This is only direct reference to a provider or online store that we should have + # for backwards compatibility. if "type" not in values["online_store"]: if values["provider"] == "local": values["online_store"]["type"] = "sqlite" @@ -168,22 +136,13 @@ def _validate_online_store_config(cls, values): online_store_type = values["online_store"]["type"] - # Make sure the user hasn't provided the wrong type - assert online_store_type in ["datastore", "sqlite", "redis"] - # 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"]) - elif online_store_type == "redis": - RedisOnlineStoreConfig(**values["online_store"]) - else: - raise ValueError(f"Invalid online store type {online_store_type}") + online_config_class = get_online_config_from_type(online_store_type) + online_config_class(**values["online_store"]) except ValidationError as e: raise ValidationError( - [ErrorWrapper(e, loc="online_store")], model=SqliteOnlineStoreConfig, + [ErrorWrapper(e, loc="online_store")], model=RepoConfig, ) return values @@ -203,7 +162,7 @@ def _validate_offline_store_config(cls, values): # Set the default type if "type" not in values["offline_store"]: - if values["provider"] == "local" or values["provider"] == "redis": + if values["provider"] == "local": values["offline_store"]["type"] = "file" elif values["provider"] == "gcp": values["offline_store"]["type"] = "bigquery" @@ -246,6 +205,38 @@ def __repr__(self) -> str: ) +def get_online_config_from_type(online_store_type: str): + if online_store_type in ONLINE_CONFIG_CLASS_FOR_TYPE: + online_store_type = ONLINE_CONFIG_CLASS_FOR_TYPE[online_store_type] + module_name, class_name = online_store_type.rsplit(".", 1) + + if not class_name.endswith("OnlineStore"): + raise errors.FeastOnlineStoreConfigInvalidName(class_name) + config_class_name = f"{class_name}Config" + + # Try importing the module that contains the custom provider + try: + module = importlib.import_module(module_name) + except Exception as e: + # The original exception can be anything - either module not found, + # or any other kind of error happening during the module import time. + # So we should include the original error as well in the stack trace. + raise errors.FeastModuleImportError( + module_name, module_type="OnlineStore" + ) from e + + # Try getting the provider class definition + try: + online_store_config_class = getattr(module, config_class_name) + except AttributeError: + # This can only be one type of error, when class_name attribute does not exist in the module + # So we don't have to include the original exception here + raise errors.FeastClassImportError( + module_name, config_class_name, class_type="OnlineStoreConfig" + ) from None + return online_store_config_class + + def load_repo_config(repo_path: Path) -> RepoConfig: config_path = repo_path / "feature_store.yaml" diff --git a/sdk/python/feast/repo_operations.py b/sdk/python/feast/repo_operations.py index 63ed5c74d72..f0489e5bb19 100644 --- a/sdk/python/feast/repo_operations.py +++ b/sdk/python/feast/repo_operations.py @@ -177,7 +177,7 @@ def apply_total(repo_config: RepoConfig, repo_path: Path): for table in repo.feature_tables: registry.apply_feature_table(table, project) click.echo( - f"Registered feature table {Style.BRIGHT + Fore.GREEN}{registry_table.name}{Style.RESET_ALL}" + f"Registered feature table {Style.BRIGHT + Fore.GREEN}{table.name}{Style.RESET_ALL}" ) # Delete views that should not exist diff --git a/sdk/python/telemetry_tests/test_telemetry.py b/sdk/python/telemetry_tests/test_telemetry.py index be6f2cb6219..9b35bf3c17a 100644 --- a/sdk/python/telemetry_tests/test_telemetry.py +++ b/sdk/python/telemetry_tests/test_telemetry.py @@ -15,15 +15,15 @@ import uuid from datetime import datetime +from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig from tenacity import retry, wait_exponential, stop_after_attempt from google.cloud import bigquery import os from time import sleep -from importlib import reload from feast import Client, Entity, ValueType, FeatureStore, RepoConfig -from feast.repo_config import SqliteOnlineStoreConfig + TELEMETRY_BIGQUERY_TABLE = ( "kf-feast.feast_telemetry.cloudfunctions_googleapis_com_cloud_functions" diff --git a/sdk/python/tests/foo_provider.py b/sdk/python/tests/foo_provider.py index 8b7e5f4d368..38367c31799 100644 --- a/sdk/python/tests/foo_provider.py +++ b/sdk/python/tests/foo_provider.py @@ -54,8 +54,8 @@ def materialize_single_feature_view( ) -> None: pass - @staticmethod def get_historical_features( + self, config: RepoConfig, feature_views: List[FeatureView], feature_refs: List[str], diff --git a/sdk/python/tests/test_feature_store.py b/sdk/python/tests/test_feature_store.py index e1e82a9ec27..49a3a9a63b0 100644 --- a/sdk/python/tests/test_feature_store.py +++ b/sdk/python/tests/test_feature_store.py @@ -17,11 +17,6 @@ import pytest from pytest_lazyfixture import lazy_fixture -from utils.data_source_utils import ( - prep_file_source, - simple_bq_source_using_query_arg, - simple_bq_source_using_table_ref_arg, -) from feast.data_format import ParquetFormat from feast.data_source import FileSource @@ -29,9 +24,15 @@ from feast.feature import Feature from feast.feature_store import FeatureStore from feast.feature_view import FeatureView +from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig from feast.protos.feast.types import Value_pb2 as ValueProto -from feast.repo_config import RepoConfig, SqliteOnlineStoreConfig +from feast.repo_config import RepoConfig from feast.value_type import ValueType +from tests.utils.data_source_utils import ( + prep_file_source, + simple_bq_source_using_query_arg, + simple_bq_source_using_table_ref_arg, +) @pytest.fixture diff --git a/sdk/python/tests/test_historical_retrieval.py b/sdk/python/tests/test_historical_retrieval.py index 83f48ccd961..00e356ae08c 100644 --- a/sdk/python/tests/test_historical_retrieval.py +++ b/sdk/python/tests/test_historical_retrieval.py @@ -14,18 +14,15 @@ from pytz import utc import feast.driver_test_data as driver_data -from feast import errors, utils +from feast import RepoConfig, errors, utils from feast.data_source import BigQuerySource, FileSource from feast.entity import Entity from feast.feature import Feature from feast.feature_store import FeatureStore from feast.feature_view import FeatureView +from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig from feast.infra.provider import DEFAULT_ENTITY_DF_EVENT_TIMESTAMP_COL -from feast.repo_config import ( - BigQueryOfflineStoreConfig, - RepoConfig, - SqliteOnlineStoreConfig, -) +from feast.repo_config import BigQueryOfflineStoreConfig from feast.value_type import ValueType np.random.seed(0) diff --git a/sdk/python/tests/test_offline_online_store_consistency.py b/sdk/python/tests/test_offline_online_store_consistency.py index 02943fd2eb8..d30541d8e70 100644 --- a/sdk/python/tests/test_offline_online_store_consistency.py +++ b/sdk/python/tests/test_offline_online_store_consistency.py @@ -17,13 +17,10 @@ from feast.feature import Feature from feast.feature_store import FeatureStore from feast.feature_view import FeatureView -from feast.repo_config import ( - DatastoreOnlineStoreConfig, - RedisOnlineStoreConfig, - RedisType, - RepoConfig, - SqliteOnlineStoreConfig, -) +from feast.infra.online_stores.datastore import DatastoreOnlineStoreConfig +from feast.infra.online_stores.redis import RedisOnlineStoreConfig, RedisType +from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig +from feast.repo_config import RepoConfig from feast.value_type import ValueType