From b7e52cb94e74a06510b179c17fa2a2c8d3c3a9aa Mon Sep 17 00:00:00 2001 From: meiravgri <109056284+meiravgri@users.noreply.github.com> Date: Tue, 12 Sep 2023 11:24:59 +0300 Subject: [PATCH 01/12] Support query timeout = 0 In RediSearch query timeout = 0 means unlimited timeout. In the current implementation, the query timeout is only updated `if timeout` which in case of 0, translates to false. Since the default timeout of the `query` class is None, replacing the condition with `if self._timeout is not None` will also work for 0. If the parameter is not a positive integer, redis server will raise an exception. related issue: #2928 https://github.com/redis/redis-py/issues/2839 --- redis/commands/search/query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/commands/search/query.py b/redis/commands/search/query.py index 5071cfabf2..362dd6c72a 100644 --- a/redis/commands/search/query.py +++ b/redis/commands/search/query.py @@ -194,7 +194,7 @@ def _get_args_tags(self): args += self._ids if self._slop >= 0: args += ["SLOP", self._slop] - if self._timeout: + if self._timeout is not None: args += ["TIMEOUT", self._timeout] if self._in_order: args.append("INORDER") From b59492b5934e44ace7a9681f0e68f88ea66f5153 Mon Sep 17 00:00:00 2001 From: meiravgri <109056284+meiravgri@users.noreply.github.com> Date: Tue, 12 Sep 2023 11:40:37 +0300 Subject: [PATCH 02/12] added a test to quety.timeout(0) --- tests/test_search.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_search.py b/tests/test_search.py index f3f9619d92..7612332470 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -2264,6 +2264,8 @@ def test_withsuffixtrie(client: redis.Redis): def test_query_timeout(r: redis.Redis): q1 = Query("foo").timeout(5000) assert q1.get_args() == ["foo", "TIMEOUT", 5000, "LIMIT", 0, 10] + q1 = Query("foo").timeout(0) + assert q1.get_args() == ["foo", "TIMEOUT", 0, "LIMIT", 0, 10] q2 = Query("foo").timeout("not_a_number") with pytest.raises(redis.ResponseError): r.ft().search(q2) From 3fb2c683ee0d481d72dc1ffcbb6ab2a5233b5a9d Mon Sep 17 00:00:00 2001 From: meiravgri <109056284+meiravgri@users.noreply.github.com> Date: Tue, 12 Sep 2023 13:18:54 +0300 Subject: [PATCH 03/12] raise an exception if query TIMEOUT is a non negative integer --- redis/commands/search/query.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/redis/commands/search/query.py b/redis/commands/search/query.py index 362dd6c72a..02bcafc7c5 100644 --- a/redis/commands/search/query.py +++ b/redis/commands/search/query.py @@ -194,8 +194,10 @@ def _get_args_tags(self): args += self._ids if self._slop >= 0: args += ["SLOP", self._slop] - if self._timeout is not None: + if isinstance(self._timeout, int) and self._timeout >= 0: args += ["TIMEOUT", self._timeout] + else: + raise AttributeError("TIMEOUT requires a non negative integer.") if self._in_order: args.append("INORDER") if self._return_fields: From 25d4ddf580f58788f7ff2cff9f4931cb483ae01b Mon Sep 17 00:00:00 2001 From: meiravgri <109056284+meiravgri@users.noreply.github.com> Date: Tue, 12 Sep 2023 13:24:02 +0300 Subject: [PATCH 04/12] fixed the query test to catach AttributeError --- tests/test_search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_search.py b/tests/test_search.py index 7612332470..a5f04a9694 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -2267,5 +2267,5 @@ def test_query_timeout(r: redis.Redis): q1 = Query("foo").timeout(0) assert q1.get_args() == ["foo", "TIMEOUT", 0, "LIMIT", 0, 10] q2 = Query("foo").timeout("not_a_number") - with pytest.raises(redis.ResponseError): + with pytest.raises(AttributeError): r.ft().search(q2) From 7a020bd836f8cd3c5906e129a71584a2959ec70a Mon Sep 17 00:00:00 2001 From: meiravgri <109056284+meiravgri@users.noreply.github.com> Date: Tue, 12 Sep 2023 13:29:07 +0300 Subject: [PATCH 05/12] Moved validating timeout to timeout() Raise the exception when the timeout is set instead of when the query is performed --- redis/commands/search/query.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/redis/commands/search/query.py b/redis/commands/search/query.py index 02bcafc7c5..f6b1f0e669 100644 --- a/redis/commands/search/query.py +++ b/redis/commands/search/query.py @@ -134,7 +134,10 @@ def slop(self, slop): def timeout(self, timeout): """overrides the timeout parameter of the module""" - self._timeout = timeout + if isinstance(timeout, int) and timeout >= 0: + self._timeout = timeou + else: + raise AttributeError("TIMEOUT requires a non negative integer.") return self def in_order(self): @@ -194,10 +197,8 @@ def _get_args_tags(self): args += self._ids if self._slop >= 0: args += ["SLOP", self._slop] - if isinstance(self._timeout, int) and self._timeout >= 0: + if self._timeout is not None: args += ["TIMEOUT", self._timeout] - else: - raise AttributeError("TIMEOUT requires a non negative integer.") if self._in_order: args.append("INORDER") if self._return_fields: From 65901308369342d781a42a287734c6d8b02bc062 Mon Sep 17 00:00:00 2001 From: meiravgri <109056284+meiravgri@users.noreply.github.com> Date: Tue, 12 Sep 2023 13:31:23 +0300 Subject: [PATCH 06/12] updates test to catch the exception when query.timeout() is called --- tests/test_search.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_search.py b/tests/test_search.py index a5f04a9694..9e08bfa8f9 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -2266,6 +2266,5 @@ def test_query_timeout(r: redis.Redis): assert q1.get_args() == ["foo", "TIMEOUT", 5000, "LIMIT", 0, 10] q1 = Query("foo").timeout(0) assert q1.get_args() == ["foo", "TIMEOUT", 0, "LIMIT", 0, 10] - q2 = Query("foo").timeout("not_a_number") with pytest.raises(AttributeError): - r.ft().search(q2) + Query("foo").timeout("not_a_number") From fb2b710ce9d579309bffac75c84041833c70055f Mon Sep 17 00:00:00 2001 From: meiravgri <109056284+meiravgri@users.noreply.github.com> Date: Tue, 12 Sep 2023 14:04:26 +0300 Subject: [PATCH 07/12] Update redis/commands/search/query.py Co-authored-by: GuyAv46 <47632673+GuyAv46@users.noreply.github.com> --- redis/commands/search/query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/commands/search/query.py b/redis/commands/search/query.py index f6b1f0e669..5271915d18 100644 --- a/redis/commands/search/query.py +++ b/redis/commands/search/query.py @@ -135,7 +135,7 @@ def slop(self, slop): def timeout(self, timeout): """overrides the timeout parameter of the module""" if isinstance(timeout, int) and timeout >= 0: - self._timeout = timeou + self._timeout = timeout else: raise AttributeError("TIMEOUT requires a non negative integer.") return self From 3d06d8069cc7e2984e78364092de730f7d61a318 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Thu, 14 Sep 2023 07:45:13 +0300 Subject: [PATCH 08/12] Revert "Update redis/commands/search/query.py" This reverts commit fb2b710ce9d579309bffac75c84041833c70055f. --- redis/commands/search/query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/commands/search/query.py b/redis/commands/search/query.py index 5271915d18..f6b1f0e669 100644 --- a/redis/commands/search/query.py +++ b/redis/commands/search/query.py @@ -135,7 +135,7 @@ def slop(self, slop): def timeout(self, timeout): """overrides the timeout parameter of the module""" if isinstance(timeout, int) and timeout >= 0: - self._timeout = timeout + self._timeout = timeou else: raise AttributeError("TIMEOUT requires a non negative integer.") return self From e4a0f895ff87c40c664937a0fc2cba636efd7c78 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Thu, 14 Sep 2023 07:45:27 +0300 Subject: [PATCH 09/12] Revert "updates test to catch the exception when query.timeout() is called" This reverts commit 65901308369342d781a42a287734c6d8b02bc062. --- tests/test_search.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_search.py b/tests/test_search.py index 9e08bfa8f9..a5f04a9694 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -2266,5 +2266,6 @@ def test_query_timeout(r: redis.Redis): assert q1.get_args() == ["foo", "TIMEOUT", 5000, "LIMIT", 0, 10] q1 = Query("foo").timeout(0) assert q1.get_args() == ["foo", "TIMEOUT", 0, "LIMIT", 0, 10] + q2 = Query("foo").timeout("not_a_number") with pytest.raises(AttributeError): - Query("foo").timeout("not_a_number") + r.ft().search(q2) From e2c7dd7ee0cdaec05a7de9302b924c4df58146f2 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Thu, 14 Sep 2023 07:45:36 +0300 Subject: [PATCH 10/12] Revert "Moved validating timeout to timeout()" This reverts commit 7a020bd836f8cd3c5906e129a71584a2959ec70a. --- redis/commands/search/query.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/redis/commands/search/query.py b/redis/commands/search/query.py index f6b1f0e669..02bcafc7c5 100644 --- a/redis/commands/search/query.py +++ b/redis/commands/search/query.py @@ -134,10 +134,7 @@ def slop(self, slop): def timeout(self, timeout): """overrides the timeout parameter of the module""" - if isinstance(timeout, int) and timeout >= 0: - self._timeout = timeou - else: - raise AttributeError("TIMEOUT requires a non negative integer.") + self._timeout = timeout return self def in_order(self): @@ -197,8 +194,10 @@ def _get_args_tags(self): args += self._ids if self._slop >= 0: args += ["SLOP", self._slop] - if self._timeout is not None: + if isinstance(self._timeout, int) and self._timeout >= 0: args += ["TIMEOUT", self._timeout] + else: + raise AttributeError("TIMEOUT requires a non negative integer.") if self._in_order: args.append("INORDER") if self._return_fields: From 0acc0c7e51c138354d56a8ff00183a3a4d36bb99 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Thu, 14 Sep 2023 07:45:40 +0300 Subject: [PATCH 11/12] Revert "fixed the query test to catach AttributeError" This reverts commit 25d4ddf580f58788f7ff2cff9f4931cb483ae01b. --- tests/test_search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_search.py b/tests/test_search.py index a5f04a9694..7612332470 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -2267,5 +2267,5 @@ def test_query_timeout(r: redis.Redis): q1 = Query("foo").timeout(0) assert q1.get_args() == ["foo", "TIMEOUT", 0, "LIMIT", 0, 10] q2 = Query("foo").timeout("not_a_number") - with pytest.raises(AttributeError): + with pytest.raises(redis.ResponseError): r.ft().search(q2) From e5ca6e3f226aec8109e0a630ab3e5afcfdb3864d Mon Sep 17 00:00:00 2001 From: meiravgri Date: Thu, 14 Sep 2023 07:45:48 +0300 Subject: [PATCH 12/12] Revert "raise an exception if query TIMEOUT is a non negative integer" This reverts commit 3fb2c683ee0d481d72dc1ffcbb6ab2a5233b5a9d. --- redis/commands/search/query.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/redis/commands/search/query.py b/redis/commands/search/query.py index 02bcafc7c5..362dd6c72a 100644 --- a/redis/commands/search/query.py +++ b/redis/commands/search/query.py @@ -194,10 +194,8 @@ def _get_args_tags(self): args += self._ids if self._slop >= 0: args += ["SLOP", self._slop] - if isinstance(self._timeout, int) and self._timeout >= 0: + if self._timeout is not None: args += ["TIMEOUT", self._timeout] - else: - raise AttributeError("TIMEOUT requires a non negative integer.") if self._in_order: args.append("INORDER") if self._return_fields: