From fe441c1d5938e5cc384c6185dc2ab237c91e4ce8 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 15 Jun 2021 16:52:20 -0700 Subject: [PATCH 01/11] Refactor OnlineStoreConfig classes into owning modules Signed-off-by: Achal Shah --- sdk/python/feast/errors.py | 26 +++-- .../feast/infra/online_stores/datastore.py | 23 +++- .../feast/infra/online_stores/helpers.py | 37 +------ .../feast/infra/online_stores/sqlite.py | 14 ++- sdk/python/feast/infra/provider.py | 4 +- sdk/python/feast/repo_config.py | 101 +++++++++++------- sdk/python/tests/test_feature_store.py | 5 +- sdk/python/tests/test_historical_retrieval.py | 2 +- .../test_offline_online_store_consistency.py | 4 +- sdk/python/tests/test_repo_config.py | 5 +- 10 files changed, 131 insertions(+), 90 deletions(-) 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..149adeb548c 100644 --- a/sdk/python/feast/infra/online_stores/datastore.py +++ b/sdk/python/feast/infra/online_stores/datastore.py @@ -24,7 +24,9 @@ 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 RepoConfig, FeastConfigBaseModel +from pydantic import StrictStr, PositiveInt +from pydantic.typing import Literal 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..dd4df47a06e 100644 --- a/sdk/python/feast/infra/online_stores/helpers.py +++ b/sdk/python/feast/infra/online_stores/helpers.py @@ -1,18 +1,13 @@ import struct -from typing import Any, Dict, Set import mmh3 -from feast.data_source import BigQuerySource, DataSource, FileSource -from feast.errors import FeastOnlineStoreUnsupportedDataSource 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, ) @@ -21,11 +16,11 @@ def get_online_store_from_config( ) -> OnlineStore: """Get the offline store from offline store config""" - if isinstance(online_store_config, SqliteOnlineStoreConfig): + if online_store_config.__repr_name__() == "SqliteOnlineStoreConfig": from feast.infra.online_stores.sqlite import SqliteOnlineStore return SqliteOnlineStore() - elif isinstance(online_store_config, DatastoreOnlineStoreConfig): + elif online_store_config.__repr_name__() == "DatastoreOnlineStoreConfig": from feast.infra.online_stores.datastore import DatastoreOnlineStore return DatastoreOnlineStore() @@ -33,33 +28,7 @@ def get_online_store_from_config( 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__ - ) + raise ValueError(f"Unsupported online store config '{online_store_config}'") def _redis_key(project: str, entity_key: EntityKeyProto): diff --git a/sdk/python/feast/infra/online_stores/sqlite.py b/sdk/python/feast/infra/online_stores/sqlite.py index 9c2e5cd251a..963ddd00fe9 100644 --- a/sdk/python/feast/infra/online_stores/sqlite.py +++ b/sdk/python/feast/infra/online_stores/sqlite.py @@ -26,7 +26,19 @@ 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 RepoConfig, FeastConfigBaseModel +from pydantic import StrictStr +from pydantic.schema import Literal + + +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): diff --git a/sdk/python/feast/infra/provider.py b/sdk/python/feast/infra/provider.py index 5d4f8d6cf0c..e23c21d5fe5 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,7 +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( + raise errors.FeastClassImportError( module_name, class_name ) from None diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index 7e70efb799b..58308c97759 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -1,10 +1,11 @@ +import importlib from enum import Enum from pathlib import Path +from typing import TypeVar, Generic, Any import yaml from pydantic import ( BaseModel, - PositiveInt, StrictInt, StrictStr, ValidationError, @@ -14,6 +15,16 @@ from pydantic.typing import Dict, Literal, Optional, Union from feast.telemetry import log_exceptions +from feast import errors + + +# 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' +} class FeastBaseModel(BaseModel): @@ -21,36 +32,18 @@ class FeastBaseModel(BaseModel): 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""" + extra = "allow" - path: StrictStr = "data/online.db" - """ (optional) Path to sqlite db """ +class FeastConfigBaseModel(BaseModel): + """ Feast Pydantic Configuration Class """ -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 """ + class Config: + arbitrary_types_allowed = True + extra = "forbid" - 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""" +OnlineT = TypeVar('OnlineT', bound=FeastConfigBaseModel) class RedisType(str, Enum): @@ -72,9 +65,7 @@ class RedisOnlineStoreConfig(FeastBaseModel): format: host:port,parameter1,parameter2 eg. redis:6379,db=0 """ -OnlineStoreConfig = Union[ - DatastoreOnlineStoreConfig, SqliteOnlineStoreConfig, RedisOnlineStoreConfig -] +OnlineStoreConfig = Union[RedisOnlineStoreConfig] class FileOfflineStoreConfig(FeastBaseModel): @@ -125,7 +116,7 @@ class RepoConfig(FeastBaseModel): provider: StrictStr """ str: local or gcp or redis """ - online_store: OnlineStoreConfig = SqliteOnlineStoreConfig() + online_store: Any """ OnlineStoreConfig: Online store configuration (optional depending on provider) """ offline_store: OfflineStoreConfig = FileOfflineStoreConfig() @@ -168,22 +159,17 @@ 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": + if 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 @@ -246,6 +232,36 @@ 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" @@ -254,6 +270,11 @@ def load_repo_config(repo_path: Path) -> RepoConfig: try: c = RepoConfig(**raw_config) c.repo_path = repo_path + online_config_class = get_online_config_from_type(c.dict()['online_store']['type']) + c.online_store = online_config_class(**c.dict()['online_store']) return c except ValidationError as e: raise FeastConfigError(e, config_path) + + + diff --git a/sdk/python/tests/test_feature_store.py b/sdk/python/tests/test_feature_store.py index e1e82a9ec27..84290f5cd9d 100644 --- a/sdk/python/tests/test_feature_store.py +++ b/sdk/python/tests/test_feature_store.py @@ -16,8 +16,9 @@ from tempfile import mkstemp import pytest +from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig from pytest_lazyfixture import lazy_fixture -from utils.data_source_utils import ( +from tests.utils.data_source_utils import ( prep_file_source, simple_bq_source_using_query_arg, simple_bq_source_using_table_ref_arg, @@ -30,7 +31,7 @@ from feast.feature_store import FeatureStore from feast.feature_view import FeatureView 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 diff --git a/sdk/python/tests/test_historical_retrieval.py b/sdk/python/tests/test_historical_retrieval.py index 83f48ccd961..cc68344a42f 100644 --- a/sdk/python/tests/test_historical_retrieval.py +++ b/sdk/python/tests/test_historical_retrieval.py @@ -9,6 +9,7 @@ import numpy as np import pandas as pd import pytest +from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig from google.cloud import bigquery from pandas.testing import assert_frame_equal from pytz import utc @@ -24,7 +25,6 @@ from feast.repo_config import ( BigQueryOfflineStoreConfig, RepoConfig, - SqliteOnlineStoreConfig, ) from feast.value_type import ValueType diff --git a/sdk/python/tests/test_offline_online_store_consistency.py b/sdk/python/tests/test_offline_online_store_consistency.py index 02943fd2eb8..de90d2e02eb 100644 --- a/sdk/python/tests/test_offline_online_store_consistency.py +++ b/sdk/python/tests/test_offline_online_store_consistency.py @@ -8,6 +8,8 @@ import pandas as pd import pytest +from feast.infra.online_stores.datastore import DatastoreOnlineStoreConfig +from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig from google.cloud import bigquery from pytz import timezone, utc @@ -18,11 +20,9 @@ from feast.feature_store import FeatureStore from feast.feature_view import FeatureView from feast.repo_config import ( - DatastoreOnlineStoreConfig, RedisOnlineStoreConfig, RedisType, RepoConfig, - SqliteOnlineStoreConfig, ) from feast.value_type import ValueType diff --git a/sdk/python/tests/test_repo_config.py b/sdk/python/tests/test_repo_config.py index 19c8ee4dcc0..a45b9924ef7 100644 --- a/sdk/python/tests/test_repo_config.py +++ b/sdk/python/tests/test_repo_config.py @@ -3,6 +3,7 @@ from textwrap import dedent from typing import Optional +import pytest from feast.repo_config import FeastConfigError, load_repo_config @@ -24,8 +25,10 @@ def _test_config(config_text, expect_error: Optional[str]): error = e if expect_error is not None: + print(f"Error: {error}") assert expect_error in str(error) else: + print(f"Error: {error}") assert error is None @@ -99,7 +102,7 @@ def test_bad_type(): path: 100500 """ ), - expect_error="__root__ -> online_store -> path\n str type expected", + expect_error="path\n str type expected", ) From f79a3b9cdd41b869de5f885906eb569f0b462799 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 15 Jun 2021 16:53:42 -0700 Subject: [PATCH 02/11] make format Signed-off-by: Achal Shah --- .../feast/infra/online_stores/datastore.py | 6 ++-- .../feast/infra/online_stores/helpers.py | 5 +-- .../feast/infra/online_stores/sqlite.py | 6 ++-- sdk/python/feast/infra/provider.py | 4 +-- sdk/python/feast/repo_config.py | 34 ++++++++----------- sdk/python/tests/test_feature_store.py | 12 +++---- sdk/python/tests/test_historical_retrieval.py | 7 ++-- .../test_offline_online_store_consistency.py | 10 ++---- sdk/python/tests/test_repo_config.py | 1 - 9 files changed, 33 insertions(+), 52 deletions(-) diff --git a/sdk/python/feast/infra/online_stores/datastore.py b/sdk/python/feast/infra/online_stores/datastore.py index 149adeb548c..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,9 +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 RepoConfig, FeastConfigBaseModel -from pydantic import StrictStr, PositiveInt -from pydantic.typing import Literal +from feast.repo_config import FeastConfigBaseModel, RepoConfig try: from google.auth.exceptions import DefaultCredentialsError diff --git a/sdk/python/feast/infra/online_stores/helpers.py b/sdk/python/feast/infra/online_stores/helpers.py index dd4df47a06e..d008e37fe1d 100644 --- a/sdk/python/feast/infra/online_stores/helpers.py +++ b/sdk/python/feast/infra/online_stores/helpers.py @@ -5,10 +5,7 @@ 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 ( - OnlineStoreConfig, - RedisOnlineStoreConfig, -) +from feast.repo_config import OnlineStoreConfig, RedisOnlineStoreConfig def get_online_store_from_config( diff --git a/sdk/python/feast/infra/online_stores/sqlite.py b/sdk/python/feast/infra/online_stores/sqlite.py index 963ddd00fe9..6c9a6c0ccaa 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,9 +28,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 RepoConfig, FeastConfigBaseModel -from pydantic import StrictStr -from pydantic.schema import Literal +from feast.repo_config import FeastConfigBaseModel, RepoConfig class SqliteOnlineStoreConfig(FeastConfigBaseModel): diff --git a/sdk/python/feast/infra/provider.py b/sdk/python/feast/infra/provider.py index e23c21d5fe5..9fe95132510 100644 --- a/sdk/python/feast/infra/provider.py +++ b/sdk/python/feast/infra/provider.py @@ -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.FeastClassImportError( - 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 58308c97759..5e39d978c0b 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -1,29 +1,22 @@ import importlib from enum import Enum from pathlib import Path -from typing import TypeVar, Generic, Any +from typing import Any, TypeVar import yaml -from pydantic import ( - BaseModel, - 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.telemetry import log_exceptions 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' + "sqlite": "feast.infra.online_stores.sqlite.SqliteOnlineStore", + "datastore": "feast.infra.online_stores.datastore.DatastoreOnlineStore", } @@ -43,7 +36,7 @@ class Config: extra = "forbid" -OnlineT = TypeVar('OnlineT', bound=FeastConfigBaseModel) +OnlineT = TypeVar("OnlineT", bound=FeastConfigBaseModel) class RedisType(str, Enum): @@ -237,7 +230,7 @@ def get_online_config_from_type(online_store_type: str): 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'): + if not class_name.endswith("OnlineStore"): raise errors.FeastOnlineStoreConfigInvalidName(class_name) config_class_name = f"{class_name}Config" @@ -248,7 +241,9 @@ def get_online_config_from_type(online_store_type: str): # 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 + raise errors.FeastModuleImportError( + module_name, module_type="OnlineStore" + ) from e # Try getting the provider class definition try: @@ -270,11 +265,10 @@ def load_repo_config(repo_path: Path) -> RepoConfig: try: c = RepoConfig(**raw_config) c.repo_path = repo_path - online_config_class = get_online_config_from_type(c.dict()['online_store']['type']) - c.online_store = online_config_class(**c.dict()['online_store']) + online_config_class = get_online_config_from_type( + c.dict()["online_store"]["type"] + ) + c.online_store = online_config_class(**c.dict()["online_store"]) return c except ValidationError as e: raise FeastConfigError(e, config_path) - - - diff --git a/sdk/python/tests/test_feature_store.py b/sdk/python/tests/test_feature_store.py index 84290f5cd9d..49a3a9a63b0 100644 --- a/sdk/python/tests/test_feature_store.py +++ b/sdk/python/tests/test_feature_store.py @@ -16,13 +16,7 @@ from tempfile import mkstemp import pytest -from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig from pytest_lazyfixture import lazy_fixture -from tests.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 @@ -30,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 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 cc68344a42f..98f9245fedc 100644 --- a/sdk/python/tests/test_historical_retrieval.py +++ b/sdk/python/tests/test_historical_retrieval.py @@ -9,7 +9,6 @@ import numpy as np import pandas as pd import pytest -from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig from google.cloud import bigquery from pandas.testing import assert_frame_equal from pytz import utc @@ -21,11 +20,9 @@ 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, -) +from feast.repo_config import BigQueryOfflineStoreConfig, RepoConfig 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 de90d2e02eb..945739130f3 100644 --- a/sdk/python/tests/test_offline_online_store_consistency.py +++ b/sdk/python/tests/test_offline_online_store_consistency.py @@ -8,8 +8,6 @@ import pandas as pd import pytest -from feast.infra.online_stores.datastore import DatastoreOnlineStoreConfig -from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig from google.cloud import bigquery from pytz import timezone, utc @@ -19,11 +17,9 @@ from feast.feature import Feature from feast.feature_store import FeatureStore from feast.feature_view import FeatureView -from feast.repo_config import ( - RedisOnlineStoreConfig, - RedisType, - RepoConfig, -) +from feast.infra.online_stores.datastore import DatastoreOnlineStoreConfig +from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig +from feast.repo_config import RedisOnlineStoreConfig, RedisType, RepoConfig from feast.value_type import ValueType diff --git a/sdk/python/tests/test_repo_config.py b/sdk/python/tests/test_repo_config.py index a45b9924ef7..a2f698c2dee 100644 --- a/sdk/python/tests/test_repo_config.py +++ b/sdk/python/tests/test_repo_config.py @@ -3,7 +3,6 @@ from textwrap import dedent from typing import Optional -import pytest from feast.repo_config import FeastConfigError, load_repo_config From 587b981ef001d9451d7ae68d54f73d81c1d7048d Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 15 Jun 2021 17:32:41 -0700 Subject: [PATCH 03/11] Move redis too Signed-off-by: Achal Shah --- .../feast/infra/online_stores/helpers.py | 44 ++++++++++++------- sdk/python/feast/infra/online_stores/redis.py | 24 +++++++++- sdk/python/feast/repo_config.py | 39 +++------------- .../test_offline_online_store_consistency.py | 3 +- 4 files changed, 58 insertions(+), 52 deletions(-) diff --git a/sdk/python/feast/infra/online_stores/helpers.py b/sdk/python/feast/infra/online_stores/helpers.py index d008e37fe1d..71693ce1354 100644 --- a/sdk/python/feast/infra/online_stores/helpers.py +++ b/sdk/python/feast/infra/online_stores/helpers.py @@ -1,31 +1,41 @@ +import importlib import struct +from typing import Any import mmh3 +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 OnlineStoreConfig, RedisOnlineStoreConfig -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 online_store_config.__repr_name__() == "SqliteOnlineStoreConfig": - from feast.infra.online_stores.sqlite import SqliteOnlineStore - - return SqliteOnlineStore() - elif online_store_config.__repr_name__() == "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 online store config '{online_store_config}'") + 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..8427f7f22c6 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 diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index 5e39d978c0b..9f1fe8fe371 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -1,7 +1,6 @@ import importlib -from enum import Enum from pathlib import Path -from typing import Any, TypeVar +from typing import Any import yaml from pydantic import BaseModel, StrictInt, StrictStr, ValidationError, root_validator @@ -17,6 +16,7 @@ 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", } @@ -36,31 +36,6 @@ class Config: extra = "forbid" -OnlineT = TypeVar("OnlineT", bound=FeastConfigBaseModel) - - -class RedisType(str, Enum): - redis = "redis" - redis_cluster = "redis_cluster" - - -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 """ - - -OnlineStoreConfig = Union[RedisOnlineStoreConfig] - - class FileOfflineStoreConfig(FeastBaseModel): """ Offline store config for local (file-based) store """ @@ -144,6 +119,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" @@ -154,13 +131,9 @@ def _validate_online_store_config(cls, values): # Validate the dict to ensure one of the union types match try: - if online_store_type == "redis": - RedisOnlineStoreConfig(**values["online_store"]) - else: - online_config_class = get_online_config_from_type(online_store_type) - online_config_class(**values["online_store"]) + 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=RepoConfig, ) diff --git a/sdk/python/tests/test_offline_online_store_consistency.py b/sdk/python/tests/test_offline_online_store_consistency.py index 945739130f3..d30541d8e70 100644 --- a/sdk/python/tests/test_offline_online_store_consistency.py +++ b/sdk/python/tests/test_offline_online_store_consistency.py @@ -18,8 +18,9 @@ from feast.feature_store import FeatureStore from feast.feature_view import FeatureView 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 RedisOnlineStoreConfig, RedisType, RepoConfig +from feast.repo_config import RepoConfig from feast.value_type import ValueType From b9bc19b81be3a934945d55a363a6790cb3ae9dbc Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 15 Jun 2021 17:38:11 -0700 Subject: [PATCH 04/11] update test_telemetery Signed-off-by: Achal Shah --- sdk/python/telemetry_tests/test_telemetry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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" From 37b5dd891e763e11b4d0d7ccb2ec0559a293ebec Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Wed, 16 Jun 2021 10:17:45 -0700 Subject: [PATCH 05/11] add a create_repo_config method that should be called instead of RepoConfig ctor directly Signed-off-by: Achal Shah --- sdk/python/feast/repo_config.py | 12 +++++++++++- sdk/python/tests/test_feature_store.py | 4 ++-- sdk/python/tests/test_historical_retrieval.py | 10 +++++----- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index 9f1fe8fe371..b527f18c70a 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -155,7 +155,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" @@ -198,6 +198,16 @@ def __repr__(self) -> str: ) +def create_repo_config(**data) -> RepoConfig: + rc = RepoConfig(**data) + if rc.online_store is None or isinstance(rc.online_store, Dict): + if rc.provider == "local": + rc.online_store = get_online_config_from_type("sqlite")() + if rc.provider == "gcp": + rc.online_store = get_online_config_from_type("datastore")() + return rc + + 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] diff --git a/sdk/python/tests/test_feature_store.py b/sdk/python/tests/test_feature_store.py index 49a3a9a63b0..497503922e2 100644 --- a/sdk/python/tests/test_feature_store.py +++ b/sdk/python/tests/test_feature_store.py @@ -26,7 +26,7 @@ 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 +from feast.repo_config import RepoConfig, create_repo_config from feast.value_type import ValueType from tests.utils.data_source_utils import ( prep_file_source, @@ -64,7 +64,7 @@ def feature_store_with_gcs_registry(): bucket.blob("registry.db") return FeatureStore( - config=RepoConfig( + config=create_repo_config( registry=f"gs://{bucket_name}/registry.db", project="default", provider="gcp", diff --git a/sdk/python/tests/test_historical_retrieval.py b/sdk/python/tests/test_historical_retrieval.py index 98f9245fedc..66105e02b72 100644 --- a/sdk/python/tests/test_historical_retrieval.py +++ b/sdk/python/tests/test_historical_retrieval.py @@ -22,7 +22,7 @@ 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 +from feast.repo_config import BigQueryOfflineStoreConfig, create_repo_config from feast.value_type import ValueType np.random.seed(0) @@ -270,7 +270,7 @@ def test_historical_features_from_parquet_sources(infer_event_timestamp_col): customer = Entity(name="customer_id", value_type=ValueType.INT64) store = FeatureStore( - config=RepoConfig( + config=create_repo_config( registry=os.path.join(temp_dir, "registry.db"), project="default", provider="local", @@ -375,7 +375,7 @@ def test_historical_features_from_bigquery_sources( if provider_type == "local": store = FeatureStore( - config=RepoConfig( + config=create_repo_config( registry=os.path.join(temp_dir, "registry.db"), project="default", provider="local", @@ -389,7 +389,7 @@ def test_historical_features_from_bigquery_sources( ) elif provider_type == "gcp": store = FeatureStore( - config=RepoConfig( + config=create_repo_config( registry=os.path.join(temp_dir, "registry.db"), project="".join( random.choices(string.ascii_uppercase + string.digits, k=10) @@ -402,7 +402,7 @@ def test_historical_features_from_bigquery_sources( ) elif provider_type == "gcp_custom_offline_config": store = FeatureStore( - config=RepoConfig( + config=create_repo_config( registry=os.path.join(temp_dir, "registry.db"), project="".join( random.choices(string.ascii_uppercase + string.digits, k=10) From fa5fd81cadd6e44eb11feb161a4fa54967162694 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Wed, 16 Jun 2021 10:19:08 -0700 Subject: [PATCH 06/11] fix the table reference in repo_operations Signed-off-by: Achal Shah --- sdk/python/feast/repo_operations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 32dce48f631db6515f421e31e9bb9e33aad0eb7d Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Wed, 16 Jun 2021 11:11:21 -0700 Subject: [PATCH 07/11] reuse create_repo_config Signed-off-by: Achal Shah Remove redis provider reference --- sdk/python/feast/infra/online_stores/sqlite.py | 3 +++ sdk/python/feast/repo_config.py | 17 ++++++----------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/sdk/python/feast/infra/online_stores/sqlite.py b/sdk/python/feast/infra/online_stores/sqlite.py index 6c9a6c0ccaa..284c5819a3a 100644 --- a/sdk/python/feast/infra/online_stores/sqlite.py +++ b/sdk/python/feast/infra/online_stores/sqlite.py @@ -77,6 +77,9 @@ def online_write_batch( ], progress: Optional[Callable[[int], Any]], ) -> None: + + print(f"online_write_batch: {config}") + conn = self._get_conn(config) project = config.project diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index b527f18c70a..c24a25dc045 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -82,7 +82,7 @@ class RepoConfig(FeastBaseModel): """ provider: StrictStr - """ str: local or gcp or redis """ + """ str: local or gcp """ online_store: Any """ OnlineStoreConfig: Online store configuration (optional depending on provider) """ @@ -200,11 +200,10 @@ def __repr__(self) -> str: def create_repo_config(**data) -> RepoConfig: rc = RepoConfig(**data) - if rc.online_store is None or isinstance(rc.online_store, Dict): - if rc.provider == "local": - rc.online_store = get_online_config_from_type("sqlite")() - if rc.provider == "gcp": - rc.online_store = get_online_config_from_type("datastore")() + if isinstance(rc.online_store, Dict): + rc.online_store = get_online_config_from_type(rc.online_store["type"])( + **rc.online_store + ) return rc @@ -246,12 +245,8 @@ def load_repo_config(repo_path: Path) -> RepoConfig: with open(config_path) as f: raw_config = yaml.safe_load(f) try: - c = RepoConfig(**raw_config) + c = create_repo_config(**raw_config) c.repo_path = repo_path - online_config_class = get_online_config_from_type( - c.dict()["online_store"]["type"] - ) - c.online_store = online_config_class(**c.dict()["online_store"]) return c except ValidationError as e: raise FeastConfigError(e, config_path) From 10601705a787b3dd39787d8ae2fff33514e42f8f Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Thu, 17 Jun 2021 15:24:23 -0700 Subject: [PATCH 08/11] CR comments Signed-off-by: Achal Shah --- sdk/python/feast/infra/online_stores/sqlite.py | 2 -- sdk/python/tests/foo_provider.py | 2 +- sdk/python/tests/test_repo_config.py | 4 +--- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/sdk/python/feast/infra/online_stores/sqlite.py b/sdk/python/feast/infra/online_stores/sqlite.py index 284c5819a3a..7385ae25a94 100644 --- a/sdk/python/feast/infra/online_stores/sqlite.py +++ b/sdk/python/feast/infra/online_stores/sqlite.py @@ -78,8 +78,6 @@ def online_write_batch( progress: Optional[Callable[[int], Any]], ) -> None: - print(f"online_write_batch: {config}") - conn = self._get_conn(config) project = config.project 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_repo_config.py b/sdk/python/tests/test_repo_config.py index a2f698c2dee..19c8ee4dcc0 100644 --- a/sdk/python/tests/test_repo_config.py +++ b/sdk/python/tests/test_repo_config.py @@ -24,10 +24,8 @@ def _test_config(config_text, expect_error: Optional[str]): error = e if expect_error is not None: - print(f"Error: {error}") assert expect_error in str(error) else: - print(f"Error: {error}") assert error is None @@ -101,7 +99,7 @@ def test_bad_type(): path: 100500 """ ), - expect_error="path\n str type expected", + expect_error="__root__ -> online_store -> path\n str type expected", ) From 39be4986bf390fe69050f7074363d594b98cc4d0 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Thu, 17 Jun 2021 15:39:58 -0700 Subject: [PATCH 09/11] Remove create_repo_config in favor of __init__ Signed-off-by: Achal Shah --- sdk/python/feast/repo_config.py | 18 ++++++++---------- sdk/python/tests/test_feature_store.py | 4 ++-- sdk/python/tests/test_historical_retrieval.py | 12 ++++++------ 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index c24a25dc045..25c55dcc2bc 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -92,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) @@ -198,15 +205,6 @@ def __repr__(self) -> str: ) -def create_repo_config(**data) -> RepoConfig: - rc = RepoConfig(**data) - if isinstance(rc.online_store, Dict): - rc.online_store = get_online_config_from_type(rc.online_store["type"])( - **rc.online_store - ) - return rc - - 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] @@ -245,7 +243,7 @@ def load_repo_config(repo_path: Path) -> RepoConfig: with open(config_path) as f: raw_config = yaml.safe_load(f) try: - c = create_repo_config(**raw_config) + c = RepoConfig(**raw_config) c.repo_path = repo_path return c except ValidationError as e: diff --git a/sdk/python/tests/test_feature_store.py b/sdk/python/tests/test_feature_store.py index 497503922e2..49a3a9a63b0 100644 --- a/sdk/python/tests/test_feature_store.py +++ b/sdk/python/tests/test_feature_store.py @@ -26,7 +26,7 @@ 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, create_repo_config +from feast.repo_config import RepoConfig from feast.value_type import ValueType from tests.utils.data_source_utils import ( prep_file_source, @@ -64,7 +64,7 @@ def feature_store_with_gcs_registry(): bucket.blob("registry.db") return FeatureStore( - config=create_repo_config( + config=RepoConfig( registry=f"gs://{bucket_name}/registry.db", project="default", provider="gcp", diff --git a/sdk/python/tests/test_historical_retrieval.py b/sdk/python/tests/test_historical_retrieval.py index 66105e02b72..df5e6cc9284 100644 --- a/sdk/python/tests/test_historical_retrieval.py +++ b/sdk/python/tests/test_historical_retrieval.py @@ -14,7 +14,7 @@ from pytz import utc import feast.driver_test_data as driver_data -from feast import errors, utils +from feast import errors, utils, RepoConfig from feast.data_source import BigQuerySource, FileSource from feast.entity import Entity from feast.feature import Feature @@ -22,7 +22,7 @@ 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, create_repo_config +from feast.repo_config import BigQueryOfflineStoreConfig from feast.value_type import ValueType np.random.seed(0) @@ -270,7 +270,7 @@ def test_historical_features_from_parquet_sources(infer_event_timestamp_col): customer = Entity(name="customer_id", value_type=ValueType.INT64) store = FeatureStore( - config=create_repo_config( + config=RepoConfig( registry=os.path.join(temp_dir, "registry.db"), project="default", provider="local", @@ -375,7 +375,7 @@ def test_historical_features_from_bigquery_sources( if provider_type == "local": store = FeatureStore( - config=create_repo_config( + config=RepoConfig( registry=os.path.join(temp_dir, "registry.db"), project="default", provider="local", @@ -389,7 +389,7 @@ def test_historical_features_from_bigquery_sources( ) elif provider_type == "gcp": store = FeatureStore( - config=create_repo_config( + config=RepoConfig( registry=os.path.join(temp_dir, "registry.db"), project="".join( random.choices(string.ascii_uppercase + string.digits, k=10) @@ -402,7 +402,7 @@ def test_historical_features_from_bigquery_sources( ) elif provider_type == "gcp_custom_offline_config": store = FeatureStore( - config=create_repo_config( + config=RepoConfig( registry=os.path.join(temp_dir, "registry.db"), project="".join( random.choices(string.ascii_uppercase + string.digits, k=10) From 25c9a4584b6c13d163d1c220601c0a692159d582 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Thu, 17 Jun 2021 15:49:48 -0700 Subject: [PATCH 10/11] make format Signed-off-by: Achal Shah --- sdk/python/tests/test_historical_retrieval.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/tests/test_historical_retrieval.py b/sdk/python/tests/test_historical_retrieval.py index df5e6cc9284..00e356ae08c 100644 --- a/sdk/python/tests/test_historical_retrieval.py +++ b/sdk/python/tests/test_historical_retrieval.py @@ -14,7 +14,7 @@ from pytz import utc import feast.driver_test_data as driver_data -from feast import errors, utils, RepoConfig +from feast import RepoConfig, errors, utils from feast.data_source import BigQuerySource, FileSource from feast.entity import Entity from feast.feature import Feature From 3ee91859055404686b8da3411a147e70e6aeca78 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Fri, 18 Jun 2021 17:51:32 -0700 Subject: [PATCH 11/11] Remove print statement Signed-off-by: Achal Shah --- sdk/python/feast/infra/online_stores/redis.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/python/feast/infra/online_stores/redis.py b/sdk/python/feast/infra/online_stores/redis.py index 8427f7f22c6..bb85a8e853d 100644 --- a/sdk/python/feast/infra/online_stores/redis.py +++ b/sdk/python/feast/infra/online_stores/redis.py @@ -121,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)