From e93439467fde67e5002e80a48e7ed8199b448c94 Mon Sep 17 00:00:00 2001 From: pyalex Date: Thu, 3 Feb 2022 16:39:44 +0800 Subject: [PATCH 1/4] debug flaky types test Signed-off-by: pyalex --- .../integration/registration/test_universal_types.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/sdk/python/tests/integration/registration/test_universal_types.py b/sdk/python/tests/integration/registration/test_universal_types.py index 663ba55ccb..7e10334624 100644 --- a/sdk/python/tests/integration/registration/test_universal_types.py +++ b/sdk/python/tests/integration/registration/test_universal_types.py @@ -226,7 +226,9 @@ def test_feature_get_online_features_types_match(online_types_test_fixtures): driver_id_value = "1" if config.entity_type == ValueType.STRING else 1 online_features = fs.get_online_features( features=features, entity_rows=[{"driver": driver_id_value}], - ).to_dict() + ) + print(online_features.proto) # will be in the logs when test fails + online_features = online_features.to_dict() feature_list_dtype_to_expected_online_response_value_type = { "int32": int, @@ -241,7 +243,10 @@ def test_feature_get_online_features_types_match(online_types_test_fixtures): ] if config.feature_is_list: for feature in online_features["value"]: - assert isinstance(feature, list) + assert isinstance(feature, list), "Feature value should be a list" + assert ( + config.has_empty_list or len(feature) > 0 + ), "List of values should not be empty" for element in feature: assert isinstance(element, expected_dtype) else: From 87dee6facbdd40a91067d42ccf823d54292893a2 Mon Sep 17 00:00:00 2001 From: pyalex Date: Thu, 3 Feb 2022 17:34:26 +0800 Subject: [PATCH 2/4] wait for table to be actually deleted Signed-off-by: pyalex --- .../tests/integration/online_store/test_universal_online.py | 1 + 1 file changed, 1 insertion(+) 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 f483d54f6b..06e6ffddb3 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -569,6 +569,7 @@ def test_online_store_cleanup(environment, universal_data_sources): assert np.allclose(expected_values["value"], online_features["value"]) fs.apply(objects=[], objects_to_delete=[simple_driver_fv], partial=False) + time.sleep(1) # some online stores have eventual consistency of schema fs.apply([simple_driver_fv]) online_features = fs.get_online_features( From 7863907f9fbff6584c770f98f47413c527ecfa22 Mon Sep 17 00:00:00 2001 From: pyalex Date: Thu, 3 Feb 2022 23:04:11 +0800 Subject: [PATCH 3/4] refresh end & start dates for each new environment instance Signed-off-by: pyalex --- .../tests/integration/feature_repos/repo_configuration.py | 7 ++----- .../tests/integration/registration/test_universal_types.py | 3 +++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index a9953d5977..7de2effc5d 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -4,7 +4,7 @@ import re import tempfile import uuid -from dataclasses import dataclass, field +from dataclasses import dataclass from datetime import datetime, timedelta from pathlib import Path from typing import Any, Dict, List, Optional, Union @@ -245,11 +245,8 @@ class Environment: python_feature_server: bool worker_id: str - end_date: datetime = field( - default=datetime.utcnow().replace(microsecond=0, second=0, minute=0) - ) - def __post_init__(self): + self.end_date = datetime.utcnow().replace(microsecond=0, second=0, minute=0) self.start_date: datetime = self.end_date - timedelta(days=3) def get_feature_server_endpoint(self) -> str: diff --git a/sdk/python/tests/integration/registration/test_universal_types.py b/sdk/python/tests/integration/registration/test_universal_types.py index 7e10334624..b8f26e092b 100644 --- a/sdk/python/tests/integration/registration/test_universal_types.py +++ b/sdk/python/tests/integration/registration/test_universal_types.py @@ -241,6 +241,9 @@ def test_feature_get_online_features_types_match(online_types_test_fixtures): expected_dtype = feature_list_dtype_to_expected_online_response_value_type[ config.feature_dtype ] + + assert len(online_features["value"]) == 1 + if config.feature_is_list: for feature in online_features["value"]: assert isinstance(feature, list), "Feature value should be a list" From 105d014053e287f2f6fa9608927baf2da72b0f77 Mon Sep 17 00:00:00 2001 From: pyalex Date: Fri, 4 Feb 2022 12:19:22 +0800 Subject: [PATCH 4/4] fix tests Signed-off-by: pyalex --- sdk/python/feast/type_map.py | 11 +++++++---- .../online_store/test_universal_online.py | 18 +++++++++++++++--- .../registration/test_universal_types.py | 11 +++++++---- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/sdk/python/feast/type_map.py b/sdk/python/feast/type_map.py index 15ddd14739..82827bce2a 100644 --- a/sdk/python/feast/type_map.py +++ b/sdk/python/feast/type_map.py @@ -300,9 +300,6 @@ def _python_value_to_proto_value( """ # ToDo: make a better sample for type checks (more than one element) sample = next(filter(_non_empty_value, values), None) # first not empty value - if sample is None: - # all input values are None or empty lists - return [ProtoValue()] * len(values) # Detect list type and handle separately if "list" in feast_value_type.name.lower(): @@ -312,7 +309,9 @@ def _python_value_to_proto_value( feast_value_type ] - if not all(type(item) in valid_types for item in sample): + if sample is not None and not all( + type(item) in valid_types for item in sample + ): first_invalid = next( item for item in sample if type(item) not in valid_types ) @@ -337,6 +336,10 @@ def _python_value_to_proto_value( # Handle scalar types below else: + if sample is None: + # all input values are None + return [ProtoValue()] * len(values) + if feast_value_type == ValueType.UNIX_TIMESTAMP: int_timestamps = _python_datetime_to_int_timestamp(values) # ProtoValue does actually accept `np.int_` but the typing complains. 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 06e6ffddb3..d81eabec39 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -4,19 +4,21 @@ import time import unittest from datetime import timedelta -from typing import Any, Dict, List, Union +from typing import Any, Dict, List, Tuple, Union import assertpy import numpy as np import pandas as pd import pytest import requests +from botocore.exceptions import BotoCoreError from feast import Entity, Feature, FeatureService, FeatureView, ValueType from feast.errors import ( FeatureNameCollisionError, RequestDataNotFoundInEntityRowsException, ) +from feast.wait import wait_retry_backoff from tests.integration.feature_repos.repo_configuration import ( Environment, construct_universal_feature_views, @@ -569,8 +571,18 @@ def test_online_store_cleanup(environment, universal_data_sources): assert np.allclose(expected_values["value"], online_features["value"]) fs.apply(objects=[], objects_to_delete=[simple_driver_fv], partial=False) - time.sleep(1) # some online stores have eventual consistency of schema - fs.apply([simple_driver_fv]) + + def eventually_apply() -> Tuple[None, bool]: + try: + fs.apply([simple_driver_fv]) + except BotoCoreError: + return None, False + + return None, True + + # Online store backend might have eventual consistency in schema update + # So recreating table that was just deleted might need some retries + wait_retry_backoff(eventually_apply, timeout_secs=60) online_features = fs.get_online_features( features=features, entity_rows=entity_rows diff --git a/sdk/python/tests/integration/registration/test_universal_types.py b/sdk/python/tests/integration/registration/test_universal_types.py index b8f26e092b..59ca119f98 100644 --- a/sdk/python/tests/integration/registration/test_universal_types.py +++ b/sdk/python/tests/integration/registration/test_universal_types.py @@ -221,14 +221,17 @@ def test_feature_get_online_features_types_match(online_types_test_fixtures): features = [fv.name + ":value"] entity = driver(value_type=config.entity_type) fs.apply([fv, entity]) - fs.materialize(environment.start_date, environment.end_date) + fs.materialize( + environment.start_date, + environment.end_date + - timedelta(hours=1) # throwing out last record to make sure + # we can successfully infer type even from all empty values + ) driver_id_value = "1" if config.entity_type == ValueType.STRING else 1 online_features = fs.get_online_features( features=features, entity_rows=[{"driver": driver_id_value}], - ) - print(online_features.proto) # will be in the logs when test fails - online_features = online_features.to_dict() + ).to_dict() feature_list_dtype_to_expected_online_response_value_type = { "int32": int,