From 80b58e45fef5cc3b2f84ec924e398dea7316f85e Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 5 Apr 2022 14:16:14 -0700 Subject: [PATCH 01/12] ci: Use testcontainers to spin up redis instances for tests Signed-off-by: Achal Shah --- sdk/python/setup.py | 2 +- sdk/python/tests/conftest.py | 6 +++ .../integration_test_repo_config.py | 13 ++++--- .../feature_repos/repo_configuration.py | 38 +++++++++++++------ .../universal/data_source_creator.py | 3 ++ .../universal/data_sources/bigquery.py | 2 +- .../universal/data_sources/file.py | 2 +- .../universal/data_sources/redshift.py | 3 +- .../universal/data_sources/snowflake.py | 3 +- .../data_sources/spark_data_source_creator.py | 2 +- .../universal/online_store/__init__.py | 0 .../universal/online_store/redis.py | 26 +++++++++++++ .../universal/online_store_creator.py | 14 +++++++ 13 files changed, 90 insertions(+), 24 deletions(-) create mode 100644 sdk/python/tests/integration/feature_repos/universal/online_store/__init__.py create mode 100644 sdk/python/tests/integration/feature_repos/universal/online_store/redis.py create mode 100644 sdk/python/tests/integration/feature_repos/universal/online_store_creator.py diff --git a/sdk/python/setup.py b/sdk/python/setup.py index 1853f144d9c..ee8625e39d1 100644 --- a/sdk/python/setup.py +++ b/sdk/python/setup.py @@ -126,7 +126,7 @@ "pytest-mock==1.10.4", "Sphinx!=4.0.0,<4.4.0", "sphinx-rtd-theme", - "testcontainers==3.4.2", + "testcontainers>=3.5", "adlfs==0.5.9", "firebase-admin==4.5.2", "pre-commit", diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 1254604a0be..dfbdc90ae26 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -184,6 +184,8 @@ def cleanup(): e.feature_store.teardown() if proc.is_alive(): proc.kill() + if e.online_store_creator: + e.online_store_creator.teardown() request.addfinalizer(cleanup) @@ -245,6 +247,8 @@ def go_data_sources(request, go_environment): def cleanup(): # logger.info("Running cleanup in %s, Request: %s", worker_id, request.param) go_environment.data_source_creator.teardown() + if environment.online_store_creator: + environment.online_store_creator.teardown() request.addfinalizer(cleanup) return construct_universal_test_data(go_environment) @@ -259,6 +263,8 @@ def e2e_data_sources(environment: Environment, request): def cleanup(): environment.data_source_creator.teardown() + if environment.online_store_creator: + environment.online_store_creator.teardown() request.addfinalizer(cleanup) 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 25650eced9f..cdc279d2ca7 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 @@ -1,5 +1,5 @@ from dataclasses import dataclass -from typing import Dict, Type, Union +from typing import Callable, Dict, Type, Union from tests.integration.feature_repos.universal.data_source_creator import ( DataSourceCreator, @@ -16,7 +16,7 @@ class IntegrationTestRepoConfig: """ provider: str = "local" - online_store: Union[str, Dict] = "sqlite" + online_store: Union[str, Dict, Callable] = "sqlite" offline_store_creator: Type[DataSourceCreator] = FileDataSourceCreator @@ -28,10 +28,13 @@ class IntegrationTestRepoConfig: def __repr__(self) -> str: if isinstance(self.online_store, str): online_store_type = self.online_store - elif self.online_store["type"] == "redis": - online_store_type = self.online_store.get("redis_type", "redis") + elif isinstance(self.online_store, dict): + if self.online_store["type"] == "redis": + online_store_type = self.online_store.get("redis_type", "redis") + else: + online_store_type = self.online_store["type"] else: - online_store_type = self.online_store["type"] + online_store_type = self.online_store.__name__ return ":".join( [ diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 18f1ece8eba..1df7e758150 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -8,7 +8,7 @@ from dataclasses import dataclass from datetime import datetime, timedelta from pathlib import Path -from typing import Any, List, Optional, Tuple, Union +from typing import Any, Callable, List, Optional, Tuple, Union import pandas as pd import yaml @@ -44,11 +44,16 @@ create_order_feature_view, create_pushable_feature_view, ) +from tests.integration.feature_repos.universal.online_store.redis import ( + RedisOnlineStoreCreator, +) +from tests.integration.feature_repos.universal.online_store_creator import ( + OnlineStoreCreator, +) DYNAMO_CONFIG = {"type": "dynamodb", "region": "us-west-2"} # Port 12345 will chosen as default for redis node configuration because Redis Cluster is started off of nodes # 6379 -> 6384. This causes conflicts in cli integration tests so we manually keep them separate. -REDIS_CONFIG = {"type": "redis", "connection_string": "localhost:6379,db=0"} REDIS_CLUSTER_CONFIG = { "type": "redis", "redis_type": "redis_cluster", @@ -65,13 +70,13 @@ # module will be imported and FULL_REPO_CONFIGS will be extracted from the file. DEFAULT_FULL_REPO_CONFIGS: List[IntegrationTestRepoConfig] = [ # Local configurations - IntegrationTestRepoConfig(), - IntegrationTestRepoConfig(python_feature_server=True), + # IntegrationTestRepoConfig(), + # IntegrationTestRepoConfig(python_feature_server=True), ] if os.getenv("FEAST_IS_LOCAL_TEST", "False") != "True": DEFAULT_FULL_REPO_CONFIGS.extend( [ - IntegrationTestRepoConfig(online_store=REDIS_CONFIG), + IntegrationTestRepoConfig(online_store=RedisOnlineStoreCreator), # GCP configurations IntegrationTestRepoConfig( provider="gcp", @@ -81,7 +86,7 @@ IntegrationTestRepoConfig( provider="gcp", offline_store_creator=BigQueryDataSourceCreator, - online_store=REDIS_CONFIG, + online_store=RedisOnlineStoreCreator, ), # AWS configurations IntegrationTestRepoConfig( @@ -93,13 +98,13 @@ IntegrationTestRepoConfig( provider="aws", offline_store_creator=RedshiftDataSourceCreator, - online_store=REDIS_CONFIG, + online_store=RedisOnlineStoreCreator, ), # Snowflake configurations IntegrationTestRepoConfig( provider="aws", # no list features, no feature server offline_store_creator=SnowflakeDataSourceCreator, - online_store=REDIS_CONFIG, + online_store=RedisOnlineStoreCreator, ), ] ) @@ -116,7 +121,9 @@ FULL_REPO_CONFIGS = DEFAULT_FULL_REPO_CONFIGS GO_REPO_CONFIGS = [ - IntegrationTestRepoConfig(online_store=REDIS_CONFIG, go_feature_server=True,), + IntegrationTestRepoConfig( + online_store=RedisOnlineStoreCreator, go_feature_server=True, + ), ] @@ -299,6 +306,7 @@ class Environment: data_source_creator: DataSourceCreator python_feature_server: bool worker_id: str + online_store_creator: Optional[OnlineStoreCreator] = None def __post_init__(self): self.end_date = datetime.utcnow().replace(microsecond=0, second=0, minute=0) @@ -341,9 +349,16 @@ def construct_test_environment( project = f"{test_suite_name}_{run_id}_{run_num}" offline_creator: DataSourceCreator = test_repo_config.offline_store_creator(project) - offline_store_config = offline_creator.create_offline_store_config() - online_store = test_repo_config.online_store + + if isinstance(test_repo_config.online_store, Callable): # type: ignore + online_creator = test_repo_config.online_store(project) + online_store = online_creator.create_online_store() + else: + online_creator = None + online_store = test_repo_config.online_store + + print(f"Online Store creator: {online_creator}, online store: {online_store}") repo_dir_name = tempfile.mkdtemp() @@ -392,6 +407,7 @@ def construct_test_environment( data_source_creator=offline_creator, python_feature_server=test_repo_config.python_feature_server, worker_id=worker_id, + online_store_creator=online_creator, ) return environment diff --git a/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py b/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py index 2a13cff3be8..7c58c80e490 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py @@ -9,6 +9,9 @@ class DataSourceCreator(ABC): + def __init__(self, project_name: str): + self.project_name = project_name + @abstractmethod def create_data_source( self, diff --git a/sdk/python/tests/integration/feature_repos/universal/data_sources/bigquery.py b/sdk/python/tests/integration/feature_repos/universal/data_sources/bigquery.py index cb7113bf66d..186ce8457dc 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_sources/bigquery.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_sources/bigquery.py @@ -18,8 +18,8 @@ class BigQueryDataSourceCreator(DataSourceCreator): dataset: Optional[Dataset] = None def __init__(self, project_name: str): + super().__init__(project_name) self.client = bigquery.Client() - self.project_name = project_name self.gcp_project = self.client.project self.dataset_id = f"{self.gcp_project}.{project_name}" diff --git a/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py b/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py index 4ae067728ca..20974cf4699 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py @@ -22,7 +22,7 @@ class FileDataSourceCreator(DataSourceCreator): files: List[Any] def __init__(self, project_name: str): - self.project_name = project_name + super().__init__(project_name) self.files = [] def create_data_source( diff --git a/sdk/python/tests/integration/feature_repos/universal/data_sources/redshift.py b/sdk/python/tests/integration/feature_repos/universal/data_sources/redshift.py index db007d83ad6..795fcfbbbb6 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_sources/redshift.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_sources/redshift.py @@ -19,8 +19,7 @@ class RedshiftDataSourceCreator(DataSourceCreator): tables: List[str] = [] def __init__(self, project_name: str): - super().__init__() - self.project_name = project_name + super().__init__(project_name) self.client = aws_utils.get_redshift_data_client("us-west-2") self.s3 = aws_utils.get_s3_resource("us-west-2") diff --git a/sdk/python/tests/integration/feature_repos/universal/data_sources/snowflake.py b/sdk/python/tests/integration/feature_repos/universal/data_sources/snowflake.py index 5be3b7383ee..c4852ccbc39 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_sources/snowflake.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_sources/snowflake.py @@ -20,8 +20,7 @@ class SnowflakeDataSourceCreator(DataSourceCreator): tables: List[str] = [] def __init__(self, project_name: str): - super().__init__() - self.project_name = project_name + super().__init__(project_name) self.offline_store_config = SnowflakeOfflineStoreConfig( type="snowflake.offline", account=os.environ["SNOWFLAKE_CI_DEPLOYMENT"], diff --git a/sdk/python/tests/integration/feature_repos/universal/data_sources/spark_data_source_creator.py b/sdk/python/tests/integration/feature_repos/universal/data_sources/spark_data_source_creator.py index 2724db50a69..49a4f539d6c 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_sources/spark_data_source_creator.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_sources/spark_data_source_creator.py @@ -24,6 +24,7 @@ class SparkDataSourceCreator(DataSourceCreator): spark_session = None def __init__(self, project_name: str): + super().__init__(project_name) self.spark_conf = { "master": "local[*]", "spark.ui.enabled": "false", @@ -31,7 +32,6 @@ def __init__(self, project_name: str): "spark.sql.parser.quotedRegexColumnNames": "true", "spark.sql.session.timeZone": "UTC", } - self.project_name = project_name if not self.spark_offline_store_config: self.create_offline_store_config() if not self.spark_session: diff --git a/sdk/python/tests/integration/feature_repos/universal/online_store/__init__.py b/sdk/python/tests/integration/feature_repos/universal/online_store/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/sdk/python/tests/integration/feature_repos/universal/online_store/redis.py b/sdk/python/tests/integration/feature_repos/universal/online_store/redis.py new file mode 100644 index 00000000000..aa464844d67 --- /dev/null +++ b/sdk/python/tests/integration/feature_repos/universal/online_store/redis.py @@ -0,0 +1,26 @@ +import typing + +from testcontainers.core.container import DockerContainer +from testcontainers.core.waiting_utils import wait_for_logs + +from tests.integration.feature_repos.universal.online_store_creator import ( + OnlineStoreCreator, +) + + +class RedisOnlineStoreCreator(OnlineStoreCreator): + def __init__(self, project_name: str): + super().__init__(project_name) + self.container = DockerContainer("redis").with_exposed_ports("6379") + + def create_online_store(self) -> typing.Dict[str, str]: + self.container.start() + log_string_to_wait_for = "Ready to accept connections" + wait_for_logs( + container=self.container, predicate=log_string_to_wait_for, timeout=5 + ) + exposed_port = self.container.get_exposed_port("6379") + return {"type": "redis", "connection_string": f"localhost:{exposed_port},db=0"} + + def teardown(self): + self.container.stop() diff --git a/sdk/python/tests/integration/feature_repos/universal/online_store_creator.py b/sdk/python/tests/integration/feature_repos/universal/online_store_creator.py new file mode 100644 index 00000000000..0fa0dbed3e1 --- /dev/null +++ b/sdk/python/tests/integration/feature_repos/universal/online_store_creator.py @@ -0,0 +1,14 @@ +from abc import ABC + +from feast.repo_config import FeastConfigBaseModel + + +class OnlineStoreCreator(ABC): + def __init__(self, project_name: str): + self.project_name = project_name + + def create_online_store(self) -> FeastConfigBaseModel: + ... + + def teardown(self): + ... From bb5aacda6b6b7a5622229b329fc820c07f603203 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 5 Apr 2022 14:31:41 -0700 Subject: [PATCH 02/12] fix test Signed-off-by: Achal Shah --- sdk/python/tests/integration/feature_repos/repo_configuration.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 1df7e758150..5c35e061a03 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -54,6 +54,7 @@ DYNAMO_CONFIG = {"type": "dynamodb", "region": "us-west-2"} # Port 12345 will chosen as default for redis node configuration because Redis Cluster is started off of nodes # 6379 -> 6384. This causes conflicts in cli integration tests so we manually keep them separate. +REDIS_CONFIG = {"type": "redis", "connection_string": "localhost:6379,db=0"} REDIS_CLUSTER_CONFIG = { "type": "redis", "redis_type": "redis_cluster", From aac2f077940dd3ce3b6ecc5c8693ea10a2025891 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 5 Apr 2022 14:33:55 -0700 Subject: [PATCH 03/12] fix test Signed-off-by: Achal Shah --- sdk/python/tests/conftest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index dfbdc90ae26..dee1aad52e8 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -24,6 +24,7 @@ from _pytest.nodes import Item from feast import FeatureStore +from feast.infra.online_stores.redis import RedisOnlineStoreConfig from tests.data.data_creator import create_dataset from tests.integration.feature_repos.integration_test_repo_config import ( IntegrationTestRepoConfig, @@ -32,7 +33,6 @@ FULL_REPO_CONFIGS, GO_REPO_CONFIGS, REDIS_CLUSTER_CONFIG, - REDIS_CONFIG, Environment, TestData, construct_test_environment, @@ -206,9 +206,9 @@ def cleanup(): @pytest.fixture( - params=[REDIS_CONFIG, REDIS_CLUSTER_CONFIG], + params=[RedisOnlineStoreConfig, REDIS_CLUSTER_CONFIG], scope="session", - ids=[str(c) for c in [REDIS_CONFIG, REDIS_CLUSTER_CONFIG]], + ids=[str(c) for c in [RedisOnlineStoreConfig, REDIS_CLUSTER_CONFIG]], ) def local_redis_environment(request, worker_id): e = construct_test_environment( From f6fdc509b2e2976160a6420482c2fda4813f0dc1 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 5 Apr 2022 14:55:29 -0700 Subject: [PATCH 04/12] fix type Signed-off-by: Achal Shah --- .../tests/integration/feature_repos/repo_configuration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 5c35e061a03..f3b786288ee 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -352,7 +352,7 @@ def construct_test_environment( offline_creator: DataSourceCreator = test_repo_config.offline_store_creator(project) offline_store_config = offline_creator.create_offline_store_config() - if isinstance(test_repo_config.online_store, Callable): # type: ignore + if isinstance(test_repo_config.online_store, Callable): online_creator = test_repo_config.online_store(project) online_store = online_creator.create_online_store() else: From 235b7fa2442fc07154a609bb4fb555fa44c85ca9 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 5 Apr 2022 15:39:40 -0700 Subject: [PATCH 05/12] fix type Signed-off-by: Achal Shah --- .../integration_test_repo_config.py | 10 ++++++--- .../feature_repos/repo_configuration.py | 22 +++++++++---------- 2 files changed, 18 insertions(+), 14 deletions(-) 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 cdc279d2ca7..c4df53884e2 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 @@ -1,5 +1,5 @@ from dataclasses import dataclass -from typing import Callable, Dict, Type, Union +from typing import Dict, Optional, Type, Union from tests.integration.feature_repos.universal.data_source_creator import ( DataSourceCreator, @@ -7,18 +7,22 @@ from tests.integration.feature_repos.universal.data_sources.file import ( FileDataSourceCreator, ) +from tests.integration.feature_repos.universal.online_store_creator import ( + OnlineStoreCreator, +) -@dataclass(frozen=True) +@dataclass(frozen=False) class IntegrationTestRepoConfig: """ This class should hold all possible parameters that may need to be varied by individual tests. """ provider: str = "local" - online_store: Union[str, Dict, Callable] = "sqlite" + online_store: Union[str, Dict] = "sqlite" offline_store_creator: Type[DataSourceCreator] = FileDataSourceCreator + online_store_creator: Optional[Type[OnlineStoreCreator]] = None full_feature_names: bool = True infer_features: bool = False diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index f3b786288ee..9e3c18eb80c 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -8,7 +8,7 @@ from dataclasses import dataclass from datetime import datetime, timedelta from pathlib import Path -from typing import Any, Callable, List, Optional, Tuple, Union +from typing import Any, List, Optional, Tuple, Union import pandas as pd import yaml @@ -71,13 +71,13 @@ # module will be imported and FULL_REPO_CONFIGS will be extracted from the file. DEFAULT_FULL_REPO_CONFIGS: List[IntegrationTestRepoConfig] = [ # Local configurations - # IntegrationTestRepoConfig(), - # IntegrationTestRepoConfig(python_feature_server=True), + IntegrationTestRepoConfig(), + IntegrationTestRepoConfig(python_feature_server=True), ] if os.getenv("FEAST_IS_LOCAL_TEST", "False") != "True": DEFAULT_FULL_REPO_CONFIGS.extend( [ - IntegrationTestRepoConfig(online_store=RedisOnlineStoreCreator), + IntegrationTestRepoConfig(online_store_creator=RedisOnlineStoreCreator), # GCP configurations IntegrationTestRepoConfig( provider="gcp", @@ -87,7 +87,7 @@ IntegrationTestRepoConfig( provider="gcp", offline_store_creator=BigQueryDataSourceCreator, - online_store=RedisOnlineStoreCreator, + online_store_creator=RedisOnlineStoreCreator, ), # AWS configurations IntegrationTestRepoConfig( @@ -99,13 +99,13 @@ IntegrationTestRepoConfig( provider="aws", offline_store_creator=RedshiftDataSourceCreator, - online_store=RedisOnlineStoreCreator, + online_store_creator=RedisOnlineStoreCreator, ), # Snowflake configurations IntegrationTestRepoConfig( provider="aws", # no list features, no feature server offline_store_creator=SnowflakeDataSourceCreator, - online_store=RedisOnlineStoreCreator, + online_store_creator=RedisOnlineStoreCreator, ), ] ) @@ -123,7 +123,7 @@ GO_REPO_CONFIGS = [ IntegrationTestRepoConfig( - online_store=RedisOnlineStoreCreator, go_feature_server=True, + online_store_creator=RedisOnlineStoreCreator, go_feature_server=True, ), ] @@ -352,9 +352,9 @@ def construct_test_environment( offline_creator: DataSourceCreator = test_repo_config.offline_store_creator(project) offline_store_config = offline_creator.create_offline_store_config() - if isinstance(test_repo_config.online_store, Callable): - online_creator = test_repo_config.online_store(project) - online_store = online_creator.create_online_store() + if test_repo_config.online_store_creator: + online_creator = test_repo_config.online_store_creator(project) + test_repo_config.online_store = online_creator.create_online_store() else: online_creator = None online_store = test_repo_config.online_store From 9c52468dd1470fd92616cb4a0dec9a77d818849d Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 5 Apr 2022 20:13:22 -0700 Subject: [PATCH 06/12] fix typo and repr Signed-off-by: Achal Shah --- .../integration_test_repo_config.py | 17 ++++++++++------- .../feature_repos/repo_configuration.py | 5 ++++- 2 files changed, 14 insertions(+), 8 deletions(-) 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 c4df53884e2..99e85120075 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 @@ -30,15 +30,18 @@ class IntegrationTestRepoConfig: go_feature_server: bool = False def __repr__(self) -> str: - if isinstance(self.online_store, str): - online_store_type = self.online_store - elif isinstance(self.online_store, dict): - if self.online_store["type"] == "redis": - online_store_type = self.online_store.get("redis_type", "redis") + if not self.online_store_creator: + if isinstance(self.online_store, str): + online_store_type = self.online_store + elif isinstance(self.online_store, dict): + if self.online_store["type"] == "redis": + online_store_type = self.online_store.get("redis_type", "redis") + else: + online_store_type = self.online_store["type"] else: - online_store_type = self.online_store["type"] + online_store_type = self.online_store.__name__ else: - online_store_type = self.online_store.__name__ + online_store_type = self.online_store_creator.__name__ return ":".join( [ diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 9e3c18eb80c..f5bdd72768d 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -78,6 +78,7 @@ DEFAULT_FULL_REPO_CONFIGS.extend( [ IntegrationTestRepoConfig(online_store_creator=RedisOnlineStoreCreator), + IntegrationTestRepoConfig(online_store=REDIS_CONFIG), # GCP configurations IntegrationTestRepoConfig( provider="gcp", @@ -354,7 +355,9 @@ def construct_test_environment( if test_repo_config.online_store_creator: online_creator = test_repo_config.online_store_creator(project) - test_repo_config.online_store = online_creator.create_online_store() + online_store = ( + test_repo_config.online_store + ) = online_creator.create_online_store() else: online_creator = None online_store = test_repo_config.online_store From 6f00801b36f1c4ec7c330fbbc5df161e685f6870 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 5 Apr 2022 21:53:19 -0700 Subject: [PATCH 07/12] datastore baby Signed-off-by: Achal Shah --- .../feature_repos/repo_configuration.py | 5 ++- .../universal/online_store/datastore.py | 38 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 sdk/python/tests/integration/feature_repos/universal/online_store/datastore.py diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index f5bdd72768d..115cf40bfdf 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -44,6 +44,9 @@ create_order_feature_view, create_pushable_feature_view, ) +from tests.integration.feature_repos.universal.online_store.datastore import ( + DatastoreOnlineStoreCreator, +) from tests.integration.feature_repos.universal.online_store.redis import ( RedisOnlineStoreCreator, ) @@ -83,7 +86,7 @@ IntegrationTestRepoConfig( provider="gcp", offline_store_creator=BigQueryDataSourceCreator, - online_store="datastore", + online_store_creator=DatastoreOnlineStoreCreator, ), IntegrationTestRepoConfig( provider="gcp", diff --git a/sdk/python/tests/integration/feature_repos/universal/online_store/datastore.py b/sdk/python/tests/integration/feature_repos/universal/online_store/datastore.py new file mode 100644 index 00000000000..736e7c8f5f5 --- /dev/null +++ b/sdk/python/tests/integration/feature_repos/universal/online_store/datastore.py @@ -0,0 +1,38 @@ +import os +import typing + +from google.cloud import datastore +from testcontainers.core.container import DockerContainer +from testcontainers.core.waiting_utils import wait_for_logs + +from tests.integration.feature_repos.universal.online_store_creator import ( + OnlineStoreCreator, +) + + +class DatastoreOnlineStoreCreator(OnlineStoreCreator): + def __init__(self, project_name: str): + super().__init__(project_name) + self.container = ( + DockerContainer( + "gcr.io/google.com/cloudsdktool/cloud-sdk:380.0.0-emulators" + ) + .with_command( + "gcloud beta emulators datastore start --project test-project --host-port 0.0.0.0:8081" + ) + .with_exposed_ports("8081") + ) + + def create_online_store(self) -> typing.Dict[str, str]: + self.container.start() + log_string_to_wait_for = r"\[datastore\] Dev App Server is now running" + wait_for_logs( + container=self.container, predicate=log_string_to_wait_for, timeout=5 + ) + exposed_port = self.container.get_exposed_port("8081") + os.environ[datastore.client.DATASTORE_EMULATOR_HOST] = f"0.0.0.0:{exposed_port}" + return {"type": "datastore", "project_id": "test-project"} + + def teardown(self): + del os.environ[datastore.client.DATASTORE_EMULATOR_HOST] + self.container.stop() From 50aa0edc4e24c84970d0fbf58619a2b135d3c71c Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Wed, 6 Apr 2022 12:28:33 -0700 Subject: [PATCH 08/12] Make containers optional Signed-off-by: Achal Shah --- .../feature_repos/repo_configuration.py | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 115cf40bfdf..3fdb8349847 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -80,18 +80,17 @@ if os.getenv("FEAST_IS_LOCAL_TEST", "False") != "True": DEFAULT_FULL_REPO_CONFIGS.extend( [ - IntegrationTestRepoConfig(online_store_creator=RedisOnlineStoreCreator), IntegrationTestRepoConfig(online_store=REDIS_CONFIG), # GCP configurations IntegrationTestRepoConfig( provider="gcp", offline_store_creator=BigQueryDataSourceCreator, - online_store_creator=DatastoreOnlineStoreCreator, + online_store="datastore", ), IntegrationTestRepoConfig( provider="gcp", offline_store_creator=BigQueryDataSourceCreator, - online_store_creator=RedisOnlineStoreCreator, + online_store=REDIS_CONFIG, ), # AWS configurations IntegrationTestRepoConfig( @@ -103,13 +102,13 @@ IntegrationTestRepoConfig( provider="aws", offline_store_creator=RedshiftDataSourceCreator, - online_store_creator=RedisOnlineStoreCreator, + online_store=REDIS_CONFIG, ), # Snowflake configurations IntegrationTestRepoConfig( provider="aws", # no list features, no feature server offline_store_creator=SnowflakeDataSourceCreator, - online_store_creator=RedisOnlineStoreCreator, + online_store=REDIS_CONFIG, ), ] ) @@ -125,10 +124,20 @@ else: FULL_REPO_CONFIGS = DEFAULT_FULL_REPO_CONFIGS +if os.getenv("FEAST_LOCAL_ONLINE_CONTAINER", "False").lower() == "true": + replacements = {"datastore": DatastoreOnlineStoreCreator} + replacement_dicts = [(REDIS_CONFIG, RedisOnlineStoreCreator)] + for c in FULL_REPO_CONFIGS: + if isinstance(c.online_store, dict): + for _replacement in replacement_dicts: + if c.online_store == _replacement[0]: + c.online_store_creator = _replacement[1] + elif c.online_store in replacements: + c.online_store_creator = replacements[c.online_store] + + GO_REPO_CONFIGS = [ - IntegrationTestRepoConfig( - online_store_creator=RedisOnlineStoreCreator, go_feature_server=True, - ), + IntegrationTestRepoConfig(online_store=REDIS_CONFIG, go_feature_server=True,), ] From f16ed529fea8b5de601887396bca0992aed7f4f4 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Wed, 6 Apr 2022 12:36:29 -0700 Subject: [PATCH 09/12] more optionality Signed-off-by: Achal Shah --- sdk/python/tests/conftest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index dee1aad52e8..dfbdc90ae26 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -24,7 +24,6 @@ from _pytest.nodes import Item from feast import FeatureStore -from feast.infra.online_stores.redis import RedisOnlineStoreConfig from tests.data.data_creator import create_dataset from tests.integration.feature_repos.integration_test_repo_config import ( IntegrationTestRepoConfig, @@ -33,6 +32,7 @@ FULL_REPO_CONFIGS, GO_REPO_CONFIGS, REDIS_CLUSTER_CONFIG, + REDIS_CONFIG, Environment, TestData, construct_test_environment, @@ -206,9 +206,9 @@ def cleanup(): @pytest.fixture( - params=[RedisOnlineStoreConfig, REDIS_CLUSTER_CONFIG], + params=[REDIS_CONFIG, REDIS_CLUSTER_CONFIG], scope="session", - ids=[str(c) for c in [RedisOnlineStoreConfig, REDIS_CLUSTER_CONFIG]], + ids=[str(c) for c in [REDIS_CONFIG, REDIS_CLUSTER_CONFIG]], ) def local_redis_environment(request, worker_id): e = construct_test_environment( From 95b0c1f4a0974c46e584774024b9302cb1711392 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Wed, 6 Apr 2022 14:18:02 -0700 Subject: [PATCH 10/12] makefile target and docs Signed-off-by: Achal Shah --- CONTRIBUTING.md | 18 +++++++++++++++++- Makefile | 3 +++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 77467576e4d..60860076d82 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -128,12 +128,15 @@ There are two sets of tests you can run: 2. Full integration tests (requires cloud environment setups) #### Local integration tests -To get local integration tests running, you'll need to have Redis setup: +To get local integration tests running, you'll need to have Redis and Docker setup: Redis 1. Install Redis: [Quickstart](https://redis.io/topics/quickstart) 2. Run `redis-server` +Docker: +1. Install Docker: [Get Docker](https://docs.docker.com/get-docker/) + Now run `make test-python-universal-local` #### Full integration tests @@ -161,6 +164,19 @@ To test across clouds, on top of setting up Redis, you also need GCP / AWS / Sno Then run `make test-python-integration`. Note that for Snowflake / GCP / AWS, this will create new temporary tables / datasets. +#### (Experimental) Run full integration tests against containerized services +Test across clouds requires existing accounts on GCP / AWS / Snowflake, and may incur costs when using these services. + +It's possible to run some integration tests against emulated local versions of these services, using ephemeral containers. +These tests create new temporary tables / datasets locally only, and they are cleaned up. when the containers are torn down. + +The services with containerized replacements currently implemented are: +- Datastore +- Redis + +You can run `make test-python-integration-container` to run tests against the containerized versions of dependencies. + + ## Feast Java Serving See [Java contributing guide](java/CONTRIBUTING.md) diff --git a/Makefile b/Makefile index 9358001f3ca..8d56f57516f 100644 --- a/Makefile +++ b/Makefile @@ -67,6 +67,9 @@ test-python: test-python-integration: FEAST_USAGE=False IS_TEST=True python -m pytest -n 8 --integration sdk/python/tests +test-python-integration-container: + FEAST_USAGE=False IS_TEST=True FEAST_LOCAL_ONLINE_CONTAINER=True python -m pytest -n 8 --integration sdk/python/tests + test-python-universal-contrib: PYTHONPATH='.' FULL_REPO_CONFIGS_MODULE=sdk.python.feast.infra.offline_stores.contrib.contrib_repo_configuration FEAST_USAGE=False IS_TEST=True python -m pytest -n 8 --integration --universal sdk/python/tests From 9b371333a58bc2480933cb00a66cf27dc3544ab2 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Wed, 6 Apr 2022 14:57:05 -0700 Subject: [PATCH 11/12] makefile target and docs Signed-off-by: Achal Shah --- sdk/python/tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index dfbdc90ae26..15e8ffc201e 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -247,8 +247,8 @@ def go_data_sources(request, go_environment): def cleanup(): # logger.info("Running cleanup in %s, Request: %s", worker_id, request.param) go_environment.data_source_creator.teardown() - if environment.online_store_creator: - environment.online_store_creator.teardown() + if go_environment.online_store_creator: + go_environment.online_store_creator.teardown() request.addfinalizer(cleanup) return construct_universal_test_data(go_environment) From b8c0936463dc4edc96cee9650d82fc90248755bd Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Wed, 6 Apr 2022 21:07:42 -0700 Subject: [PATCH 12/12] cr Signed-off-by: Achal Shah --- CONTRIBUTING.md | 7 +++---- .../tests/integration/feature_repos/repo_configuration.py | 2 -- .../feature_repos/universal/online_store/datastore.py | 4 ++-- .../feature_repos/universal/online_store/redis.py | 4 ++-- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 60860076d82..2a4cb74634c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -128,15 +128,12 @@ There are two sets of tests you can run: 2. Full integration tests (requires cloud environment setups) #### Local integration tests -To get local integration tests running, you'll need to have Redis and Docker setup: +To get local integration tests running, you'll need to have Redis setup: Redis 1. Install Redis: [Quickstart](https://redis.io/topics/quickstart) 2. Run `redis-server` -Docker: -1. Install Docker: [Get Docker](https://docs.docker.com/get-docker/) - Now run `make test-python-universal-local` #### Full integration tests @@ -167,6 +164,8 @@ Then run `make test-python-integration`. Note that for Snowflake / GCP / AWS, th #### (Experimental) Run full integration tests against containerized services Test across clouds requires existing accounts on GCP / AWS / Snowflake, and may incur costs when using these services. +For this approach of running tests, you'll need to have docker set up locally: [Get Docker](https://docs.docker.com/get-docker/) + It's possible to run some integration tests against emulated local versions of these services, using ephemeral containers. These tests create new temporary tables / datasets locally only, and they are cleaned up. when the containers are torn down. diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 3fdb8349847..cf53e76c58c 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -374,8 +374,6 @@ def construct_test_environment( online_creator = None online_store = test_repo_config.online_store - print(f"Online Store creator: {online_creator}, online store: {online_store}") - repo_dir_name = tempfile.mkdtemp() if test_repo_config.python_feature_server and test_repo_config.provider == "aws": diff --git a/sdk/python/tests/integration/feature_repos/universal/online_store/datastore.py b/sdk/python/tests/integration/feature_repos/universal/online_store/datastore.py index 736e7c8f5f5..52851e80d88 100644 --- a/sdk/python/tests/integration/feature_repos/universal/online_store/datastore.py +++ b/sdk/python/tests/integration/feature_repos/universal/online_store/datastore.py @@ -1,5 +1,5 @@ import os -import typing +from typing import Dict from google.cloud import datastore from testcontainers.core.container import DockerContainer @@ -23,7 +23,7 @@ def __init__(self, project_name: str): .with_exposed_ports("8081") ) - def create_online_store(self) -> typing.Dict[str, str]: + def create_online_store(self) -> Dict[str, str]: self.container.start() log_string_to_wait_for = r"\[datastore\] Dev App Server is now running" wait_for_logs( diff --git a/sdk/python/tests/integration/feature_repos/universal/online_store/redis.py b/sdk/python/tests/integration/feature_repos/universal/online_store/redis.py index aa464844d67..073760f5145 100644 --- a/sdk/python/tests/integration/feature_repos/universal/online_store/redis.py +++ b/sdk/python/tests/integration/feature_repos/universal/online_store/redis.py @@ -1,4 +1,4 @@ -import typing +from typing import Dict from testcontainers.core.container import DockerContainer from testcontainers.core.waiting_utils import wait_for_logs @@ -13,7 +13,7 @@ def __init__(self, project_name: str): super().__init__(project_name) self.container = DockerContainer("redis").with_exposed_ports("6379") - def create_online_store(self) -> typing.Dict[str, str]: + def create_online_store(self) -> Dict[str, str]: self.container.start() log_string_to_wait_for = "Ready to accept connections" wait_for_logs(