Skip to content

Commit 4ba1485

Browse files
committed
Fix the cache key generation, test_list fail and add unit tests for cache
Signed-off-by: Barry Wu <a0987818905@gmail.com>
1 parent e7e75e9 commit 4ba1485

3 files changed

Lines changed: 129 additions & 7 deletions

File tree

flytekit/core/type_engine.py

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,7 +1175,7 @@ class TypeEngine(typing.Generic[T]):
11751175
_DATACLASS_TRANSFORMER: TypeTransformer = DataclassTransformer() # type: ignore
11761176
_ENUM_TRANSFORMER: TypeTransformer = EnumTransformer() # type: ignore
11771177
lazy_import_lock = threading.Lock()
1178-
_CACHE = LRUCache(maxsize=128)
1178+
_CACHE: LRUCache = LRUCache(maxsize=128)
11791179

11801180
@classmethod
11811181
def register(
@@ -1379,9 +1379,27 @@ def calculate_hash(cls, python_val: typing.Any, python_type: Type[T]) -> Optiona
13791379
break
13801380
return hsh
13811381

1382-
@lru_cache(typed=True)
1383-
def make_key(python_val: typing.Any, python_type: Type[T]) -> tuple:
1384-
return (repr(python_val), python_type.__name__)
1382+
@classmethod
1383+
def make_key(cls, python_val: typing.Any, python_type: Type[T]) -> Optional[tuple]:
1384+
import cloudpickle
1385+
1386+
val_hash: typing.Any
1387+
type_hash: typing.Any
1388+
try:
1389+
try:
1390+
val_hash = hash(python_val)
1391+
except Exception:
1392+
val_hash = hash(cloudpickle.dumps(python_val))
1393+
1394+
try:
1395+
type_hash = hash(python_type)
1396+
except Exception:
1397+
type_hash = hash(cloudpickle.dumps(python_type))
1398+
1399+
return (val_hash, type_hash)
1400+
1401+
except Exception:
1402+
return None
13851403

13861404
@classmethod
13871405
def to_literal(
@@ -1393,7 +1411,7 @@ def to_literal(
13931411
namely an async transformer.
13941412
"""
13951413
key = cls.make_key(python_val, python_type)
1396-
if key in cls._CACHE:
1414+
if key is not None and key in cls._CACHE:
13971415
return cls._CACHE[key]
13981416

13991417
from flytekit.core.promise import Promise
@@ -1417,7 +1435,9 @@ def to_literal(
14171435
modify_literal_uris(lv)
14181436
lv.hash = cls.calculate_hash(python_val, python_type)
14191437

1420-
cls._CACHE[key] = lv
1438+
if key is not None:
1439+
cls._CACHE[key] = lv
1440+
14211441
return lv
14221442

14231443
@classmethod

tests/flytekit/unit/core/test_list.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,15 @@ async def test_coroutine_batching_of_list_transformer():
7373

7474
lt = LiteralType(simple=SimpleType.INTEGER)
7575
python_val = [MyInt(10), MyInt(11), MyInt(12), MyInt(13), MyInt(14)]
76+
# Use the different python_val to avoid hitting the cache
77+
python_val_2 = [MyInt(11), MyInt(10), MyInt(12), MyInt(13), MyInt(14)]
7678
ctx = FlyteContext.current_context()
7779

7880
with mock.patch("flytekit.core.type_engine._TYPE_ENGINE_COROS_BATCH_SIZE", 2):
7981
TypeEngine.to_literal(ctx, python_val, typing.List[MyInt], lt)
8082

8183
with mock.patch("flytekit.core.type_engine._TYPE_ENGINE_COROS_BATCH_SIZE", 5):
8284
with pytest.raises(ValueError):
83-
TypeEngine.to_literal(ctx, python_val, typing.List[MyInt], lt)
85+
TypeEngine.to_literal(ctx, python_val_2, typing.List[MyInt], lt)
8486

8587
del TypeEngine._REGISTRY[MyInt]

tests/flytekit/unit/core/test_type_engine.py

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3851,3 +3851,103 @@ async def test_dict_transformer_annotated_type():
38513851

38523852
literal3 = await TypeEngine.async_to_literal(ctx, nested_dict, nested_dict_type, expected_type)
38533853
assert literal3.map.literals["outer"].map.literals["inner"].scalar.primitive.integer == 42
3854+
3855+
def test_type_engine_cache():
3856+
# Clear cache before test
3857+
TypeEngine._CACHE.clear()
3858+
3859+
# Test data
3860+
ctx = FlyteContext.current_context()
3861+
python_val = [1, 2, 3, 4, 5]
3862+
python_type = typing.List[int]
3863+
expected = TypeEngine.to_literal_type(python_type)
3864+
3865+
# First call - should not use cache
3866+
literal1 = TypeEngine.to_literal(ctx, python_val, python_type, expected)
3867+
3868+
# Verify cache is populated
3869+
key = TypeEngine.make_key(python_val, python_type)
3870+
assert key is not None
3871+
assert key in TypeEngine._CACHE
3872+
3873+
# Second call with same parameters - should use cache
3874+
literal2 = TypeEngine.to_literal(ctx, python_val, python_type, expected)
3875+
3876+
# Verify both literals are identical (same object from cache)
3877+
assert literal1 is literal2
3878+
3879+
# Test with different data - should not use cache
3880+
different_val = [2, 1, 3, 4, 5]
3881+
literal3 = TypeEngine.to_literal(ctx, different_val, python_type, expected)
3882+
key_different = TypeEngine.make_key(different_val, python_type)
3883+
3884+
assert key_different is not key
3885+
assert key_different is not None
3886+
assert key_different in TypeEngine._CACHE
3887+
3888+
# Verify different literals are different objects
3889+
assert literal1 is not literal3
3890+
3891+
# Test cache with unhashable objects (should not cache)
3892+
python_val = {"a": [1, 2, 3]} # dict with list is unhashable
3893+
python_type = typing.Dict[str, typing.List[int]]
3894+
expected = TypeEngine.to_literal_type(python_type)
3895+
3896+
# First call
3897+
literal4 = TypeEngine.to_literal(ctx, python_val, python_type, expected)
3898+
key = TypeEngine.make_key(python_val, python_type)
3899+
assert key is not None
3900+
assert key in TypeEngine._CACHE
3901+
3902+
# Second call with same unhashable data
3903+
literal5 = TypeEngine.to_literal(ctx, python_val, python_type, expected)
3904+
3905+
# Should be different objects since unhashable objects can't be cached
3906+
assert literal4 is literal5
3907+
3908+
# Add many different values to test cache size limit
3909+
for i in range(200): # More than the default maxsize of 128
3910+
test_val = [i, i+1, i+2]
3911+
test_type = typing.List[int]
3912+
test_expected = TypeEngine.to_literal_type(test_type)
3913+
TypeEngine.to_literal(ctx, test_val, test_type, test_expected)
3914+
3915+
# Cache should not exceed maxsize
3916+
assert len(TypeEngine._CACHE) == 128
3917+
3918+
# Test cache with pandas DataFrame (if available)
3919+
try:
3920+
import pandas as pd
3921+
3922+
# Create DataFrame
3923+
df = pd.DataFrame({"a": [1, 2, 3], "b": [4, 5, 6]})
3924+
df_type = pd.DataFrame
3925+
df_expected = TypeEngine.to_literal_type(df_type)
3926+
3927+
# First call
3928+
literal6 = TypeEngine.to_literal(ctx, df, df_type, df_expected)
3929+
3930+
# Second call with same DataFrame
3931+
literal7 = TypeEngine.to_literal(ctx, df, df_type, df_expected)
3932+
3933+
# Should be same object (DataFrame should be hashable via cloudpickle)
3934+
assert literal6 is literal7
3935+
3936+
except ImportError:
3937+
# Skip pandas test if not available
3938+
pass
3939+
3940+
# Clean up
3941+
TypeEngine._CACHE.clear()
3942+
3943+
def test_make_key_with_annotated_types():
3944+
# Test with Annotated type
3945+
annotated_val = [1, 2, 3]
3946+
annotated_type = typing.Annotated[typing.List[int], "test_annotation"]
3947+
3948+
key = TypeEngine.make_key(annotated_val, annotated_type)
3949+
key_without_annotation = TypeEngine.make_key(annotated_val, typing.List[int])
3950+
# Should handle Annotated types correctly
3951+
assert key is not None
3952+
assert key_without_annotation is not None
3953+
assert key != key_without_annotation

0 commit comments

Comments
 (0)