From c243357493d2a570bac5ea5f5a349607c704a394 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Thu, 5 May 2022 17:15:28 -0700 Subject: [PATCH 1/4] chore: Make loading online and offline classes lazy Signed-off-by: Achal Shah --- .../feast/infra/passthrough_provider.py | 24 ++++++-- sdk/python/feast/repo_config.py | 55 ++++++++++++++----- .../test_dynamodb_online_store.py | 1 + 3 files changed, 59 insertions(+), 21 deletions(-) diff --git a/sdk/python/feast/infra/passthrough_provider.py b/sdk/python/feast/infra/passthrough_provider.py index 0b6b798fe01..4b2dc2a71f3 100644 --- a/sdk/python/feast/infra/passthrough_provider.py +++ b/sdk/python/feast/infra/passthrough_provider.py @@ -39,12 +39,24 @@ def __init__(self, config: RepoConfig): super().__init__(config) self.repo_config = config - self.offline_store = get_offline_store_from_config(config.offline_store) - self.online_store = ( - get_online_store_from_config(config.online_store) - if config.online_store - else None - ) + self._offline_store = None + self._online_store = None + + @property + def online_store(self): + if not self._online_store: + self._online_store = get_online_store_from_config( + self.repo_config.online_store + ) + return self._online_store + + @property + def offline_store(self): + if not self._offline_store: + self._offline_store = get_offline_store_from_config( + self.repo_config.offline_store + ) + return self._offline_store def update_infra( self, diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index ef1871f2fe6..0a874b9080a 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -6,6 +6,7 @@ import yaml from pydantic import ( BaseModel, + Field, StrictInt, StrictStr, ValidationError, @@ -107,10 +108,10 @@ class RepoConfig(FeastBaseModel): provider: StrictStr """ str: local or gcp or aws """ - online_store: Any + _online_config: Any = Field(alias="online_store") """ OnlineStoreConfig: Online store configuration (optional depending on provider) """ - offline_store: Any + _offline_config: Any = Field(alias="offline_store") """ OfflineStoreConfig: Offline store configuration (optional depending on provider) """ feature_server: Optional[Any] @@ -125,20 +126,13 @@ class RepoConfig(FeastBaseModel): def __init__(self, **data: Any): super().__init__(**data) + self._offline_store = None + if "offline_store" in data: + self._offline_config = data["offline_store"] - if isinstance(self.online_store, Dict): - self.online_store = get_online_config_from_type(self.online_store["type"])( - **self.online_store - ) - elif isinstance(self.online_store, str): - self.online_store = get_online_config_from_type(self.online_store)() - - if isinstance(self.offline_store, Dict): - self.offline_store = get_offline_config_from_type( - self.offline_store["type"] - )(**self.offline_store) - elif isinstance(self.offline_store, str): - self.offline_store = get_offline_config_from_type(self.offline_store)() + self._online_store = None + if "online_store" in data: + self._online_config = data["online_store"] if isinstance(self.feature_server, Dict): self.feature_server = get_feature_server_config_from_type( @@ -151,6 +145,34 @@ def get_registry_config(self): else: return self.registry + @property + def offline_store(self): + if not self._offline_store: + if isinstance(self._offline_config, Dict): + self._offline_store = get_offline_config_from_type( + self._offline_config["type"] + )(**self._offline_config) + elif isinstance(self._offline_config, str): + self._offline_store = get_offline_config_from_type( + self._offline_config + )() + else: + self._offline_store = self._offline_config + return self._offline_store + + @property + def online_store(self): + if not self._online_store: + if isinstance(self._online_config, Dict): + self._online_store = get_online_config_from_type( + self._online_config["type"] + )(**self._online_config) + elif isinstance(self._online_config, str): + self._online_store = get_online_config_from_type(self._online_config)() + else: + self._online_store = self._online_config + return self._online_store + @root_validator(pre=True) @log_exceptions def _validate_online_store_config(cls, values): @@ -304,6 +326,9 @@ def write_to_path(self, repo_path: Path): sort_keys=False, ) + class Config: + allow_population_by_field_name = True + class FeastConfigError(Exception): def __init__(self, error_message, config_path): diff --git a/sdk/python/tests/unit/infra/online_store/test_dynamodb_online_store.py b/sdk/python/tests/unit/infra/online_store/test_dynamodb_online_store.py index fa2bb7601b2..6275a177e05 100644 --- a/sdk/python/tests/unit/infra/online_store/test_dynamodb_online_store.py +++ b/sdk/python/tests/unit/infra/online_store/test_dynamodb_online_store.py @@ -39,6 +39,7 @@ def repo_config(): project=PROJECT, provider=PROVIDER, online_store=DynamoDBOnlineStoreConfig(region=REGION), + # online_store={"type": "dynamodb", "region": REGION}, offline_store=FileOfflineStoreConfig(), ) From be4834ad0f420fcf1bf046840af25c74bf70515f Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Thu, 5 May 2022 21:56:36 -0700 Subject: [PATCH 2/4] fixes Signed-off-by: Achal Shah --- sdk/python/feast/repo_config.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index 0a874b9080a..7043a67d161 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -126,9 +126,17 @@ class RepoConfig(FeastBaseModel): def __init__(self, **data: Any): super().__init__(**data) + self._offline_store = None if "offline_store" in data: self._offline_config = data["offline_store"] + else: + if data["provider"] == "local": + self._offline_config = "file" + elif data["provider"] == "gcp": + self._offline_config = "bigquery" + elif data["provider"] == "aws": + self._offline_config = "redshift" self._online_store = None if "online_store" in data: @@ -156,7 +164,7 @@ def offline_store(self): self._offline_store = get_offline_config_from_type( self._offline_config )() - else: + elif self._offline_config: self._offline_store = self._offline_config return self._offline_store @@ -169,8 +177,9 @@ def online_store(self): )(**self._online_config) elif isinstance(self._online_config, str): self._online_store = get_online_config_from_type(self._online_config)() - else: + elif self._online_config: self._online_store = self._online_config + return self._online_store @root_validator(pre=True) From 525e230d2234fde028b818a0d27f3f49344d3d50 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Thu, 5 May 2022 22:19:19 -0700 Subject: [PATCH 3/4] fixes Signed-off-by: Achal Shah --- sdk/python/feast/repo_config.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index 7043a67d161..62d799a2b6b 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -141,6 +141,13 @@ def __init__(self, **data: Any): self._online_store = None if "online_store" in data: self._online_config = data["online_store"] + else: + if data["provider"] == "local": + self._online_config = "sqlite" + elif data["provider"] == "gcp": + self._online_config = "datastore" + elif data["provider"] == "aws": + self._online_config = "dynamodb" if isinstance(self.feature_server, Dict): self.feature_server = get_feature_server_config_from_type( From dc791219e4b4376bf34cc6704c60b31b562e9c3e Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Fri, 6 May 2022 09:05:14 -0700 Subject: [PATCH 4/4] nullability fixes Signed-off-by: Achal Shah --- sdk/python/feast/infra/passthrough_provider.py | 2 +- .../feature_repos/integration_test_repo_config.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/sdk/python/feast/infra/passthrough_provider.py b/sdk/python/feast/infra/passthrough_provider.py index 4b2dc2a71f3..b5965c91bfb 100644 --- a/sdk/python/feast/infra/passthrough_provider.py +++ b/sdk/python/feast/infra/passthrough_provider.py @@ -44,7 +44,7 @@ def __init__(self, config: RepoConfig): @property def online_store(self): - if not self._online_store: + if not self._online_store and self.repo_config.online_store: self._online_store = get_online_store_from_config( self.repo_config.online_store ) diff --git a/sdk/python/tests/integration/feature_repos/integration_test_repo_config.py b/sdk/python/tests/integration/feature_repos/integration_test_repo_config.py index f8cd66a6196..d014719bd0a 100644 --- a/sdk/python/tests/integration/feature_repos/integration_test_repo_config.py +++ b/sdk/python/tests/integration/feature_repos/integration_test_repo_config.py @@ -19,7 +19,7 @@ class IntegrationTestRepoConfig: """ provider: str = "local" - online_store: Union[str, Dict] = "sqlite" + online_store: Optional[Union[str, Dict]] = "sqlite" offline_store_creator: Type[DataSourceCreator] = FileDataSourceCreator online_store_creator: Optional[Type[OnlineStoreCreator]] = None @@ -38,8 +38,10 @@ def __repr__(self) -> str: online_store_type = self.online_store.get("redis_type", "redis") else: online_store_type = self.online_store["type"] - else: + elif self.online_store: online_store_type = self.online_store.__name__ + else: + online_store_type = "none" else: online_store_type = self.online_store_creator.__name__