[SF-11323] disallow sending command getkeys

[bellatoris]

Pull Request check-list

Please make sure to review and check all of these items:

• Does $ tox pass with this change (including linting)?
• Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
• Is the new or changed code fully tested?
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
• Is there an example added to the examples folder (if applicable)?
• Was the change added to CHANGES file?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Description of change

Please provide a description of the change here.

[zach-iee]

LGTM

4.5 and above introduced a command packer, so we would have to rewrite the code, when we plan to upgrade to 4.5

redis-py claims that 4.5 has over 25% performance boost in write
https://github.com/redis/redis-py/releases/tag/v4.5.0

[zach-iee]

please fix the style to pass the linter
https://github.com/sendbird/redis-py/actions/runs/5542821285/jobs/10118003768?pr=7

[zach-iee]

Also tests for zdiffstore and etc are failing, because command getkeys have been disabled. We can implement the same functionality in https://github.com/sendbird/soda/blob/develop/soda/cache/connection.py as a mixin if you want

[bellatoris]

Also tests for zdiffstore and etc are failing, because command getkeys have been disabled. We can implement the same functionality in https://github.com/sendbird/soda/blob/develop/soda/cache/connection.py as a mixin if you want

I am wary about that other library which uses redis client directly crashes the redis server... So that's the why I implemented this logic here

[bellatoris]

LGTM

4.5 and above introduced a command packer, so we would have to rewrite the code, when we plan to upgrade to 4.5

redis-py claims that 4.5 has over 25% performance boost in write https://github.com/redis/redis-py/releases/tag/v4.5.0

Do we have plan to upgrade to 4.5?

[zach-iee]

LGTM
4.5 and above introduced a command packer, so we would have to rewrite the code, when we plan to upgrade to 4.5
redis-py claims that 4.5 has over 25% performance boost in write https://github.com/redis/redis-py/releases/tag/v4.5.0

Do we have plan to upgrade to 4.5?

i was working on it without a definitive due date. The current approach doesn't hurt upgradability, but just requires a code patch.

I am wary about that other library which uses redis client directly crashes the redis server... So that's the why I implemented this logic here

if that's the worry, the current code may be the best guarantee. If so, we can fix the tests that are failing in this repo :)

[bellatoris]

If so, we can fix the tests that are failing in this repo :)

hmm hard to find which tests are failing due to this patch. any idea?

[zach-iee]

Details

you can take a look at the result of the github actions

for example, https://github.com/sendbird/redis-py/actions/runs/5551140608/jobs/10136956857?pr=7 contains one of the test results. Tests that failed from E Exception: Redis command "COMMAND GETKEYS" is not supported seems to be caused by this PR

[bellatoris]

Details

you can take a look at the result of the github actions

for example, https://github.com/sendbird/redis-py/actions/runs/5551140608/jobs/10136956857?pr=7 contains one of the test results. Tests that failed from E Exception: Redis command "COMMAND GETKEYS" is not supported seems to be caused by this PR

Sigh... too many

[codecov-commenter]

Codecov Report

Patch coverage: 57.67% and project coverage change: -0.21 ⚠️

Comparison is base (8a5dada) 60.80% compared to head (c872af5) 60.60%.

Additional details and impacted files

@@            Coverage Diff             @@
##              4.4       #7      +/-   ##
==========================================
- Coverage   60.80%   60.60%   -0.21%     
==========================================
  Files         116      116              
  Lines       29840    29922      +82     
==========================================
- Hits        18144    18133      -11     
- Misses      11696    11789      +93     

Impacted Files	Coverage Δ
tests/test_commands.py	68.94% <50.82%> (-1.81%)	⬇️
tests/test_command_parser.py	84.00% <68.75%> (-9.88%)	⬇️
tests/test_cluster.py	96.51% <87.83%> (-0.22%)	⬇️
redis/connection.py	84.99% <100.00%> (+0.03%)	⬆️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[bellatoris]

@zach-iee please review one more time