From 724ebe35b5b35e25e8e0e54f81aac9412a0ad0d8 Mon Sep 17 00:00:00 2001 From: Anton Kudryavtsev Date: Thu, 11 Jul 2024 18:53:54 +0300 Subject: [PATCH 01/11] Fix default cache dataloader raise key error on non-existing key Closes #2464 --- RELEASE.md | 12 ++++++++++++ strawberry/dataloader.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 RELEASE.md diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 0000000000..bea1d7cdd5 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,12 @@ +Release type: patch + +Calling `.clean(key)` on default dataloader with non-existing `key` will not throw `KeyError` error anymore. Example: +```python +async def load_data(keys): + return [str(key) for key in keys] + +dataloader = DataLoader(load_fn=load_data) +dataloader.clean(42) # does not throw KeyError anymore +``` + +This is a patch release, so no breaking changes. \ No newline at end of file diff --git a/strawberry/dataloader.py b/strawberry/dataloader.py index a094a892e3..aeb520a1a0 100644 --- a/strawberry/dataloader.py +++ b/strawberry/dataloader.py @@ -84,7 +84,7 @@ def set(self, key: K, value: Future[T]) -> None: self.cache_map[self.cache_key_fn(key)] = value def delete(self, key: K) -> None: - del self.cache_map[self.cache_key_fn(key)] + self.cache_map.pop(self.cache_key_fn(key), default=None) def clear(self) -> None: self.cache_map.clear() From 7a2a88a5f1e34f7a67d0510099e92220e63a3ca2 Mon Sep 17 00:00:00 2001 From: Anton Kudryavtsev Date: Thu, 11 Jul 2024 19:00:04 +0300 Subject: [PATCH 02/11] Modify test to test behaviour on non-existing cache key --- tests/test_dataloaders.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_dataloaders.py b/tests/test_dataloaders.py index f00bc430f7..5e36790147 100644 --- a/tests/test_dataloaders.py +++ b/tests/test_dataloaders.py @@ -301,6 +301,8 @@ async def idx(keys: List[int]) -> List[Tuple[int, int]]: loader = DataLoader(load_fn=idx, cache=False) + loader.clean(1) # no effect on non-cached values + assert await loader.load_many([1, 2, 3]) == [(1, 1), (2, 1), (3, 1)] loader.clear(1) From 5fe7f6f97a630087c2ed76b41ba5a64c94512cce Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 11 Jul 2024 16:04:35 +0000 Subject: [PATCH 03/11] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- RELEASE.md | 5 +++-- tests/test_dataloaders.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index bea1d7cdd5..3f698b28a8 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -5,8 +5,9 @@ Calling `.clean(key)` on default dataloader with non-existing `key` will not thr async def load_data(keys): return [str(key) for key in keys] + dataloader = DataLoader(load_fn=load_data) -dataloader.clean(42) # does not throw KeyError anymore +dataloader.clean(42) # does not throw KeyError anymore ``` -This is a patch release, so no breaking changes. \ No newline at end of file +This is a patch release, so no breaking changes. diff --git a/tests/test_dataloaders.py b/tests/test_dataloaders.py index 5e36790147..8a95e1c8be 100644 --- a/tests/test_dataloaders.py +++ b/tests/test_dataloaders.py @@ -301,7 +301,7 @@ async def idx(keys: List[int]) -> List[Tuple[int, int]]: loader = DataLoader(load_fn=idx, cache=False) - loader.clean(1) # no effect on non-cached values + loader.clean(1) # no effect on non-cached values assert await loader.load_many([1, 2, 3]) == [(1, 1), (2, 1), (3, 1)] From 92a4ff00cf3b35241668a3c18cafd4e349dbd575 Mon Sep 17 00:00:00 2001 From: Anton Kudryavtsev Date: Thu, 11 Jul 2024 19:13:43 +0300 Subject: [PATCH 04/11] Update test to explicitly check no exception was raised --- tests/test_dataloaders.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_dataloaders.py b/tests/test_dataloaders.py index 8a95e1c8be..99f176b305 100644 --- a/tests/test_dataloaders.py +++ b/tests/test_dataloaders.py @@ -301,7 +301,10 @@ async def idx(keys: List[int]) -> List[Tuple[int, int]]: loader = DataLoader(load_fn=idx, cache=False) - loader.clean(1) # no effect on non-cached values + try: + loader.clean(1) # no effect on non-cached values + except Exception as e: + assert False, "clean non-cached values does not raise KeyError" assert await loader.load_many([1, 2, 3]) == [(1, 1), (2, 1), (3, 1)] From eab45a51da7be05e950cbd649181172aea7df7bc Mon Sep 17 00:00:00 2001 From: Anton Kudryavtsev Date: Thu, 11 Jul 2024 19:14:13 +0300 Subject: [PATCH 05/11] Update RELEASE.md --- RELEASE.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 3f698b28a8..cd87edc677 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -1,7 +1,9 @@ Release type: patch -Calling `.clean(key)` on default dataloader with non-existing `key` will not throw `KeyError` error anymore. Example: +Calling `.clean(key)` on default dataloader with non-existing `key` will not throw `KeyError` anymore. Example: ```python +from strawberry.dataloader import DataLoader + async def load_data(keys): return [str(key) for key in keys] @@ -10,4 +12,4 @@ dataloader = DataLoader(load_fn=load_data) dataloader.clean(42) # does not throw KeyError anymore ``` -This is a patch release, so no breaking changes. +This is a patch release with no breaking changes. From 4ac166dd7b791c79df2c5da5fed413fbf6af21f8 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 11 Jul 2024 16:14:30 +0000 Subject: [PATCH 06/11] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- RELEASE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE.md b/RELEASE.md index cd87edc677..1dc0a9c845 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -4,6 +4,7 @@ Calling `.clean(key)` on default dataloader with non-existing `key` will not thr ```python from strawberry.dataloader import DataLoader + async def load_data(keys): return [str(key) for key in keys] From c1892364199d6f2e6b112a3c5b1a5a8cee0879b9 Mon Sep 17 00:00:00 2001 From: Anton Kudryavtsev Date: Thu, 11 Jul 2024 23:10:43 +0300 Subject: [PATCH 07/11] Modify test to test behaviour on non-existing cache key --- strawberry/dataloader.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/strawberry/dataloader.py b/strawberry/dataloader.py index aeb520a1a0..c66d992176 100644 --- a/strawberry/dataloader.py +++ b/strawberry/dataloader.py @@ -84,7 +84,9 @@ def set(self, key: K, value: Future[T]) -> None: self.cache_map[self.cache_key_fn(key)] = value def delete(self, key: K) -> None: - self.cache_map.pop(self.cache_key_fn(key), default=None) + cache_key = self.cache_key_fn(key) + if cache_key in self.cache_map: + del self.cache_map[cache_key] def clear(self) -> None: self.cache_map.clear() From 5b87885b3d405b94db095a2c511bd9f81c04c1b0 Mon Sep 17 00:00:00 2001 From: Anton Kudryavtsev Date: Thu, 11 Jul 2024 23:13:57 +0300 Subject: [PATCH 08/11] Address B011 Do not assert False --- tests/test_dataloaders.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_dataloaders.py b/tests/test_dataloaders.py index 99f176b305..adee76dfb9 100644 --- a/tests/test_dataloaders.py +++ b/tests/test_dataloaders.py @@ -304,7 +304,7 @@ async def idx(keys: List[int]) -> List[Tuple[int, int]]: try: loader.clean(1) # no effect on non-cached values except Exception as e: - assert False, "clean non-cached values does not raise KeyError" + raise AssertionError("clean non-cached values does not raise KeyError") assert await loader.load_many([1, 2, 3]) == [(1, 1), (2, 1), (3, 1)] From f3b225af6216cb38b1e17aab5b1678c1f710c56b Mon Sep 17 00:00:00 2001 From: Anton Kudryavtsev Date: Mon, 21 Oct 2024 10:31:27 +0300 Subject: [PATCH 09/11] fix: typo in RELEASE.md Co-authored-by: Gary Donovan --- RELEASE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE.md b/RELEASE.md index 1dc0a9c845..ba37ca003b 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -1,6 +1,6 @@ Release type: patch -Calling `.clean(key)` on default dataloader with non-existing `key` will not throw `KeyError` anymore. Example: +Calling `.clear(key)` on default dataloader with non-existing `key` will not throw `KeyError` anymore. Example: ```python from strawberry.dataloader import DataLoader From 708013fcd64a711afae846d2f4abaf246833a595 Mon Sep 17 00:00:00 2001 From: Anton Kudryavtsev Date: Mon, 21 Oct 2024 10:31:43 +0300 Subject: [PATCH 10/11] fix: typo in RELEASE.md Co-authored-by: Gary Donovan --- RELEASE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE.md b/RELEASE.md index ba37ca003b..5184a6594b 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -10,7 +10,7 @@ async def load_data(keys): dataloader = DataLoader(load_fn=load_data) -dataloader.clean(42) # does not throw KeyError anymore +dataloader.clear(42) # does not throw KeyError anymore ``` This is a patch release with no breaking changes. From 6b0a704b97ee37128cb7e553446aa565c2cf769a Mon Sep 17 00:00:00 2001 From: Anton Kudryavtsev Date: Mon, 21 Oct 2024 10:43:50 +0300 Subject: [PATCH 11/11] test: update `test_clear` and `test_clear_nocache` to test the new cachemap deletion behaviour These tests call `.clear()` on a non-existent cache key. Previously, this would have resulted in an exception being thrown. The new behaviour should be to throw no exceptions. --- tests/test_dataloaders.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/test_dataloaders.py b/tests/test_dataloaders.py index adee76dfb9..e277e9971e 100644 --- a/tests/test_dataloaders.py +++ b/tests/test_dataloaders.py @@ -288,6 +288,10 @@ async def idx(keys: List[int]) -> List[Tuple[int, int]]: assert await loader.load_many([1, 2, 3]) == [(1, 4), (2, 4), (3, 4)] + loader.clear(-1) + + assert await loader.load_many([1, 2, 3]) == [(1, 4), (2, 4), (3, 4)] + @pytest.mark.asyncio async def test_clear_nocache(): @@ -301,11 +305,6 @@ async def idx(keys: List[int]) -> List[Tuple[int, int]]: loader = DataLoader(load_fn=idx, cache=False) - try: - loader.clean(1) # no effect on non-cached values - except Exception as e: - raise AssertionError("clean non-cached values does not raise KeyError") - assert await loader.load_many([1, 2, 3]) == [(1, 1), (2, 1), (3, 1)] loader.clear(1) @@ -320,6 +319,10 @@ async def idx(keys: List[int]) -> List[Tuple[int, int]]: assert await loader.load_many([1, 2, 3]) == [(1, 4), (2, 4), (3, 4)] + loader.clear(-1) + + assert await loader.load_many([1, 2, 3]) == [(1, 5), (2, 5), (3, 5)] + @pytest.mark.asyncio async def test_dont_dispatch_cancelled():