Fix flaky tests (test_online_store_cleanup & test_feature_get_online_features_types_match) (feast-dev#2276)

pyalex · web-flow · commit aeaeb91d1229 · 2022-02-05T16:56:12.000-08:00
* debug flaky types test

Signed-off-by: pyalex &lt;moskalenko.alexey@gmail.com&gt;

* wait for table to be actually deleted

Signed-off-by: pyalex &lt;moskalenko.alexey@gmail.com&gt;

* refresh end &amp; start dates for each new environment instance

Signed-off-by: pyalex &lt;moskalenko.alexey@gmail.com&gt;

* fix tests

Signed-off-by: pyalex &lt;moskalenko.alexey@gmail.com&gt;
diff --git 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/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/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,7 +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)
-    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
@@ -221,7 +221,12 @@ 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(
@@ -239,9 +244,15 @@ 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)
+            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:
