From a4894f8c97b3f55b8d54933d478f613c6f06cd02 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Sat, 5 Mar 2022 22:26:59 -0800 Subject: [PATCH 01/10] fix: Use the correct dockerhub image tag when building feature servers Signed-off-by: Achal Shah --- sdk/python/feast/infra/aws.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/infra/aws.py b/sdk/python/feast/infra/aws.py index 104e20388a..e01ecfcc95 100644 --- a/sdk/python/feast/infra/aws.py +++ b/sdk/python/feast/infra/aws.py @@ -262,7 +262,9 @@ def _upload_docker_image( _logger.info( f"Pulling remote image {Style.BRIGHT + Fore.GREEN}{dockerhub_image}{Style.RESET_ALL}" ) - for line in docker_client.api.pull(dockerhub_image, stream=True, decode=True): + for line in docker_client.api.pull( + dockerhub_image.replace("_", "."), stream=True, decode=True + ): _logger.debug(f" {line}") auth_token = ecr_client.get_authorization_token()["authorizationData"][0][ From 2756bce7bea305dab0c4d2bab5fe2a647cb9a2ab Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Sat, 5 Mar 2022 22:59:07 -0800 Subject: [PATCH 02/10] Print attempt Signed-off-by: Achal Shah --- .../tests/integration/online_store/test_universal_online.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index bdbdfb3555..33d97596c5 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -262,11 +262,12 @@ def _get_online_features_dict_remotely( request["features"] = features else: request["feature_service"] = features.name - for _ in range(25): + for attempt in range(25): # Send the request to the remote feature server and get the response in JSON format response = requests.post( f"{endpoint}/get-online-features", json=request, timeout=30 ).json() + print(f"Attempt: {attempt}, {response}") # Retry if the response is internal server error, which can happen when lambda is being restarted if response.get("message") != "Internal Server Error": break From 9570ab896b614ad79463f7d8460fd0fdfad6c587 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Sat, 5 Mar 2022 23:19:46 -0800 Subject: [PATCH 03/10] Increase sleep interval Signed-off-by: Achal Shah --- .../integration/online_store/test_universal_online.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index 33d97596c5..e54507b0bb 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -1,5 +1,6 @@ import datetime import itertools +import logging import os import time import unittest @@ -35,6 +36,8 @@ ) from tests.utils.data_source_utils import prep_file_source +_logger = logging.getLogger(__name__) + @pytest.mark.integration def test_entity_ttl_online_store(local_redis_environment, redis_universal_data_sources): @@ -267,12 +270,12 @@ def _get_online_features_dict_remotely( response = requests.post( f"{endpoint}/get-online-features", json=request, timeout=30 ).json() - print(f"Attempt: {attempt}, {response}") + _logger.info(f"Attempt: {attempt}, {response}") # Retry if the response is internal server error, which can happen when lambda is being restarted if response.get("message") != "Internal Server Error": break # Sleep between retries to give the server some time to start - time.sleep(1) + time.sleep(5) else: raise Exception("Failed to get online features from remote feature server") if "metadata" not in response: From 2e87f4ebe43e6ac93a900f6306e2fa40564e2061 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Mon, 7 Mar 2022 09:12:13 -0800 Subject: [PATCH 04/10] More logging and sleep Signed-off-by: Achal Shah --- sdk/python/feast/infra/aws.py | 2 ++ .../tests/integration/online_store/test_universal_online.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/infra/aws.py b/sdk/python/feast/infra/aws.py index e01ecfcc95..d957153ae2 100644 --- a/sdk/python/feast/infra/aws.py +++ b/sdk/python/feast/infra/aws.py @@ -119,6 +119,8 @@ def _deploy_feature_server(self, project: str, image_uri: str): lambda_client = boto3.client("lambda") api_gateway_client = boto3.client("apigatewayv2") function = aws_utils.get_lambda_function(lambda_client, resource_name) + _logger.info("Using function name: %s", resource_name) + _logger.info("Found function: %s", function) if function is None: # If the Lambda function does not exist, create it. diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index e54507b0bb..20c6827dc2 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -275,7 +275,7 @@ def _get_online_features_dict_remotely( if response.get("message") != "Internal Server Error": break # Sleep between retries to give the server some time to start - time.sleep(5) + time.sleep(15) else: raise Exception("Failed to get online features from remote feature server") if "metadata" not in response: From da64fbfac3fc17b1ad13a23955b77b37bd85748b Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Mon, 7 Mar 2022 09:30:49 -0800 Subject: [PATCH 05/10] change logging to debug Signed-off-by: Achal Shah --- sdk/python/feast/infra/aws.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/infra/aws.py b/sdk/python/feast/infra/aws.py index d957153ae2..4abdcfc15c 100644 --- a/sdk/python/feast/infra/aws.py +++ b/sdk/python/feast/infra/aws.py @@ -119,8 +119,8 @@ def _deploy_feature_server(self, project: str, image_uri: str): lambda_client = boto3.client("lambda") api_gateway_client = boto3.client("apigatewayv2") function = aws_utils.get_lambda_function(lambda_client, resource_name) - _logger.info("Using function name: %s", resource_name) - _logger.info("Found function: %s", function) + _logger.debug("Using function name: %s", resource_name) + _logger.debug("Found function: %s", function) if function is None: # If the Lambda function does not exist, create it. From 5a433ee5f7abffd6b098116b2096b4bbd9860ec1 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Mon, 7 Mar 2022 09:40:00 -0800 Subject: [PATCH 06/10] Change the dockerimage name Signed-off-by: Achal Shah --- sdk/python/feast/infra/aws.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/feast/infra/aws.py b/sdk/python/feast/infra/aws.py index 4abdcfc15c..0000706d21 100644 --- a/sdk/python/feast/infra/aws.py +++ b/sdk/python/feast/infra/aws.py @@ -283,7 +283,7 @@ def _upload_docker_image( ) _logger.debug(f" {login_status}") - image = docker_client.images.get(dockerhub_image) + image = docker_client.images.get(dockerhub_image.replace("_", ".")) image_remote_name = f"{repository_uri}:{docker_image_version}" _logger.info( f"Pushing local image to remote {Style.BRIGHT + Fore.GREEN}{image_remote_name}{Style.RESET_ALL}" From ee380a5e45192fde2b6787f7fb8af53ecc82be12 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Mon, 7 Mar 2022 10:00:02 -0800 Subject: [PATCH 07/10] Change version to have periods Signed-off-by: Achal Shah --- sdk/python/feast/infra/aws.py | 2 +- .../tests/integration/online_store/test_universal_online.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/sdk/python/feast/infra/aws.py b/sdk/python/feast/infra/aws.py index 0000706d21..d30674a620 100644 --- a/sdk/python/feast/infra/aws.py +++ b/sdk/python/feast/infra/aws.py @@ -288,7 +288,7 @@ def _upload_docker_image( _logger.info( f"Pushing local image to remote {Style.BRIGHT + Fore.GREEN}{image_remote_name}{Style.RESET_ALL}" ) - image.tag(image_remote_name) + image.tag(image_remote_name.replace('_', '.')) for line in docker_client.api.push( repository_uri, tag=docker_image_version, stream=True, decode=True ): diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index 20c6827dc2..688f2496d3 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -36,8 +36,6 @@ ) from tests.utils.data_source_utils import prep_file_source -_logger = logging.getLogger(__name__) - @pytest.mark.integration def test_entity_ttl_online_store(local_redis_environment, redis_universal_data_sources): @@ -270,7 +268,6 @@ def _get_online_features_dict_remotely( response = requests.post( f"{endpoint}/get-online-features", json=request, timeout=30 ).json() - _logger.info(f"Attempt: {attempt}, {response}") # Retry if the response is internal server error, which can happen when lambda is being restarted if response.get("message") != "Internal Server Error": break From fba6523c210f4fee99d62606ccb97d1483ce33d8 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Mon, 7 Mar 2022 10:05:45 -0800 Subject: [PATCH 08/10] Change version entirely Signed-off-by: Achal Shah --- sdk/python/feast/infra/aws.py | 14 ++++++-------- .../feature_repos/repo_configuration.py | 11 +++++------ .../online_store/test_universal_online.py | 1 - 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/sdk/python/feast/infra/aws.py b/sdk/python/feast/infra/aws.py index d30674a620..b55e792218 100644 --- a/sdk/python/feast/infra/aws.py +++ b/sdk/python/feast/infra/aws.py @@ -264,9 +264,7 @@ def _upload_docker_image( _logger.info( f"Pulling remote image {Style.BRIGHT + Fore.GREEN}{dockerhub_image}{Style.RESET_ALL}" ) - for line in docker_client.api.pull( - dockerhub_image.replace("_", "."), stream=True, decode=True - ): + for line in docker_client.api.pull(dockerhub_image, stream=True, decode=True): _logger.debug(f" {line}") auth_token = ecr_client.get_authorization_token()["authorizationData"][0][ @@ -283,12 +281,12 @@ def _upload_docker_image( ) _logger.debug(f" {login_status}") - image = docker_client.images.get(dockerhub_image.replace("_", ".")) + image = docker_client.images.get(dockerhub_image) image_remote_name = f"{repository_uri}:{docker_image_version}" _logger.info( f"Pushing local image to remote {Style.BRIGHT + Fore.GREEN}{image_remote_name}{Style.RESET_ALL}" ) - image.tag(image_remote_name.replace('_', '.')) + image.tag(image_remote_name) for line in docker_client.api.push( repository_uri, tag=docker_image_version, stream=True, decode=True ): @@ -313,7 +311,7 @@ def _create_or_get_repository_uri(self, ecr_client): def _get_lambda_name(project: str): lambda_prefix = AWS_LAMBDA_FEATURE_SERVER_REPOSITORY - lambda_suffix = f"{project}-{_get_docker_image_version()}" + lambda_suffix = f"{project}-{_get_docker_image_version().replace('.', '_')}" # AWS Lambda name can't have the length greater than 64 bytes. # This usually occurs during integration tests where feast version is long if len(lambda_prefix) + len(lambda_suffix) >= 63: @@ -342,7 +340,7 @@ def _get_docker_image_version() -> str: else: version = get_version() if "dev" in version: - version = version[: version.find("dev") - 1].replace(".", "_") + version = version[: version.find("dev") - 1] _logger.warning( "You are trying to use AWS Lambda feature server while Feast is in a development mode. " f"Feast will use a docker image version {version} derived from Feast SDK " @@ -352,7 +350,7 @@ def _get_docker_image_version() -> str: "> pip install -e sdk/python" ) else: - version = version.replace(".", "_") + version = version return version diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 89aea727a6..2b9ee03c98 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -47,7 +47,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:12345,db=0"} +REDIS_CONFIG = {"type": "redis", "connection_string": "localhost:6379,db=0"} REDIS_CLUSTER_CONFIG = { "type": "redis", "redis_type": "redis_cluster", @@ -70,9 +70,7 @@ if os.getenv("FEAST_IS_LOCAL_TEST", "False") != "True": DEFAULT_FULL_REPO_CONFIGS.extend( [ - # Redis configurations IntegrationTestRepoConfig(online_store=REDIS_CONFIG), - IntegrationTestRepoConfig(online_store=REDIS_CLUSTER_CONFIG), # GCP configurations IntegrationTestRepoConfig( provider="gcp", @@ -263,7 +261,7 @@ def values(self): def construct_universal_feature_views( - data_sources: UniversalDataSources, + data_sources: UniversalDataSources, with_odfv: bool = True, ) -> UniversalFeatureViews: driver_hourly_stats = create_driver_hourly_stats_feature_view(data_sources.driver) return UniversalFeatureViews( @@ -275,7 +273,9 @@ def construct_universal_feature_views( "driver": driver_hourly_stats, "input_request": create_conv_rate_request_data_source(), } - ), + ) + if with_odfv + else None, driver_age_request_fv=create_driver_age_request_feature_view(), order=create_order_feature_view(data_sources.orders), location=create_location_stats_feature_view(data_sources.location), @@ -358,7 +358,6 @@ def construct_test_environment( registry = RegistryConfig( path=str(Path(repo_dir_name) / "registry.db"), cache_ttl_seconds=1, ) - config = RepoConfig( registry=registry, project=project, diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index 688f2496d3..ede09cc2cf 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -1,6 +1,5 @@ import datetime import itertools -import logging import os import time import unittest From f459c9a95093e69e81ab8e958eeb8ecdafad861e Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Mon, 7 Mar 2022 10:35:29 -0800 Subject: [PATCH 09/10] Undo unintended changes Signed-off-by: Achal Shah --- .../tests/integration/feature_repos/repo_configuration.py | 3 ++- .../tests/integration/online_store/test_universal_online.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 2b9ee03c98..7381c76d0c 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -47,7 +47,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_CONFIG = {"type": "redis", "connection_string": "localhost:12345,db=0"} REDIS_CLUSTER_CONFIG = { "type": "redis", "redis_type": "redis_cluster", @@ -71,6 +71,7 @@ DEFAULT_FULL_REPO_CONFIGS.extend( [ IntegrationTestRepoConfig(online_store=REDIS_CONFIG), + IntegrationTestRepoConfig(online_store=REDIS_CLUSTER_CONFIG), # GCP configurations IntegrationTestRepoConfig( provider="gcp", diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index ede09cc2cf..c85d320d70 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -262,7 +262,7 @@ def _get_online_features_dict_remotely( request["features"] = features else: request["feature_service"] = features.name - for attempt in range(25): + for _ in range(25): # Send the request to the remote feature server and get the response in JSON format response = requests.post( f"{endpoint}/get-online-features", json=request, timeout=30 From 7dabf794cba2947135a1fa20905d313dff278479 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Mon, 7 Mar 2022 10:59:41 -0800 Subject: [PATCH 10/10] Remove dead branch Signed-off-by: Achal Shah --- sdk/python/feast/infra/aws.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/python/feast/infra/aws.py b/sdk/python/feast/infra/aws.py index b55e792218..b7cc61de0e 100644 --- a/sdk/python/feast/infra/aws.py +++ b/sdk/python/feast/infra/aws.py @@ -349,8 +349,6 @@ def _get_docker_image_version() -> str: "> git fetch --all --tags\n" "> pip install -e sdk/python" ) - else: - version = version return version