Truncate pipeline exception message to a sane size #3530
Conversation
|
Hi @rohansingh thank you for your contribution! Your change will be reviewed soon. |
rohansingh
force-pushed
the
truncate-pipeline-exception
branch
from
17a16c6 to
2778765
Compare
|
|
@rohansingh Hi! Looks like some cluster tests are still failing |
|
Sorry, I've had trouble getting the cluster tests to run in my dev environment for some reason. Will take a look in a bit. |
|
LGTM, just need to resolve conflicts and fix tests |
|
Yup sorry about the delay, I will get to this today! I appreciate the prompt reviews. |
rohansingh
force-pushed
the
truncate-pipeline-exception
branch
from
4599a56 to
0bd8365
Compare
There was a problem hiding this comment.
Hi @rohansingh , linters are failing and I have one more small request to rename the new function. It can truncate any text and we can use it for other purposes as well.
For the linters, have in mind that we have updated the tooling recently and you might need install again the dev dependencies.
| @@ -257,3 +257,9 @@ def ensure_string(key): | |||
| return key | |||
| else: | |||
| raise TypeError("Key must be either a string or bytes") | |||
|
|
|||
|
|
|||
| def truncate_command_for_exception(self, command, max_length=100): | |||
There was a problem hiding this comment.
This function can be used for all types of text truncation and would be better if its name is changed to something like truncate_txt, for example, and also fix the names of the used parameters.
There was a problem hiding this comment.
@rohansingh @petyaslavova textwrap.shorten()
There was a problem hiding this comment.
@excitoon That was a perfect suggestion for this case :) Thank you!
@rohansingh can you please use that function - it is included in all supported Python versions and we can use it directly in all places where we need to shorten the strings.
| @@ -1692,23 +1692,23 @@ def test_cluster_bitop_not_empty_string(self, r): | |||
|
|
|||
| @skip_if_server_version_lt("2.6.0") | |||
| def test_cluster_bitop_not(self, r): | |||
| test_str = b"\xaa\x00\xff\x55" | |||
| test_str = b"\xAA\x00\xFF\x55" | |||
There was a problem hiding this comment.
% grep -R -E '\\x[0-9a-f][a-f]|\\x[a-f][0-9a-f]' tests
Binary file tests/testdata/will_play_text.csv.bz2 matches
tests/test_encoding.py: r.set("a", b"foo\xff")
tests/test_encoding.py: r.set("a", b"foo\xff")
tests/test_commands.py: test_str = b"\xaa\x00\xff\x55"
tests/test_commands.py: test_str = b"\xaa\x00\xff\x55"
tests/test_commands.py: test_str = b"\x01\x02\xff"
tests/test_commands.py: r["a"] = b"\x01\x02\xff\xff"
tests/test_commands.py: r["b"] = b"\x01\x02\xff"
tests/test_commands.py: r.set(key, b"\xff\xf0\x00")
tests/test_commands.py: r.set(key, b"\x00\xff\xf0")
tests/test_commands.py: r.set(key, b"\xff\xf0\x00")
tests/test_commands.py: r.set("mykey", b"\x00\xff\xf0")
tests/test_cluster.py: assert r.keyslot("大奖") == r.keyslot(b"\xe5\xa4\xa7\xe5\xa5\x96")
tests/test_cluster.py: assert r.keyslot("大奖") == r.keyslot(b"\xe5\xa4\xa7\xe5\xa5\x96")
tests/test_cluster.py: test_str = b"\xaa\x00\xff\x55"
tests/test_cluster.py: test_str = b"\xaa\x00\xff\x55"
tests/test_cluster.py: test_str = b"\x01\x02\xff"
tests/test_cluster.py: r["{foo}a"] = b"\x01\x02\xff\xff"
tests/test_cluster.py: r["{foo}b"] = b"\x01\x02\xff"
Binary file tests/test_asyncio/testdata/will_play_text.csv.bz2 matches
tests/test_asyncio/test_encoding.py: await r.set("a", b"foo\xff")
tests/test_asyncio/test_encoding.py: await r.set("a", b"foo\xff")
tests/test_asyncio/test_commands.py: test_str = b"\xaa\x00\xff\x55"
tests/test_asyncio/test_commands.py: test_str = b"\xaa\x00\xff\x55"
tests/test_asyncio/test_commands.py: test_str = b"\x01\x02\xff"
tests/test_asyncio/test_commands.py: await r.set("a", b"\x01\x02\xff\xff")
tests/test_asyncio/test_commands.py: await r.set("b", b"\x01\x02\xff")
tests/test_asyncio/test_commands.py: await r.set(key, b"\xff\xf0\x00")
tests/test_asyncio/test_commands.py: await r.set(key, b"\x00\xff\xf0")
tests/test_asyncio/test_commands.py: await r.set(key, b"\xff\xf0\x00")
tests/test_asyncio/test_cluster.py: assert r.keyslot("大奖") == r.keyslot(b"\xe5\xa4\xa7\xe5\xa5\x96")
tests/test_asyncio/test_cluster.py: assert r.keyslot("大奖") == r.keyslot(b"\xe5\xa4\xa7\xe5\xa5\x96")
tests/test_asyncio/test_cluster.py: test_str = b"\xaa\x00\xff\x55"
tests/test_asyncio/test_cluster.py: test_str = b"\xaa\x00\xff\x55"
tests/test_asyncio/test_cluster.py: test_str = b"\x01\x02\xff"
tests/test_asyncio/test_cluster.py: await r.set("{foo}a", b"\x01\x02\xff\xff")
tests/test_asyncio/test_cluster.py: await r.set("{foo}b", b"\x01\x02\xff")
tests/test_asyncio/test_scripting.py: msgpack_message_1 = b"\x81\xa4name\xa3Joe"
tests/test_scripting.py: return b"\xcf\x84o\xcf\x81\xce\xbdo\xcf\x82"
tests/test_scripting.py: assert Script(r, script_bytes).script == b"\xcf\x84o\xcf\x81\xce\xbdo\xcf\x82"
tests/test_scripting.py: msgpack_message_dumped = b"\x81\xa4name\xa3Joe"
tests/test_scripting.py: msgpack_message_1 = b"\x81\xa4name\xa3Joe"
There was a problem hiding this comment.
@excitoon If you want to point out that there are more files to be fixed - I would suggest not handling those with this PR. At some point, we will update something in those files, and they will be fixed as well by the ruff autoformatting
There was a problem hiding this comment.
Not quite; I'm rather pointing that all of these things don't need to be fixed. 😄 Uppercase is way too aggressive from my perspective 😁
Fixes #3491.