Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List instead of set for SMEMBERS in 5.1 #3390

Closed
jules-ch opened this issue Sep 28, 2024 · 7 comments · Fixed by #3399
Closed

List instead of set for SMEMBERS in 5.1 #3390

jules-ch opened this issue Sep 28, 2024 · 7 comments · Fixed by #3399
Labels
bug Bug

Comments

@jules-ch
Copy link

jules-ch commented Sep 28, 2024

Version: What redis-py and what redis version is the issue happening on?

Platform: What platform / version? Python 3.11 Ubuntu 24.04

Description: smembers is returning a list instead of a set in redis-py 5.1.0 which is an unintended breaking change.
Before:

>>> from redis import Redis
>>> pool = Redis.from_url("redis://", decode_responses=True)
>>> pool.sadd("users", "user1")
0
>>> pool.smembers("users")
{'user1'}

AFter

>>> from redis import Redis
>>> pool = Redis.from_url("redis://", decode_responses=True)
>>> pool.sadd("users", "user1")
0
>>> pool.smembers("users")
['user1']
@jules-ch
Copy link
Author

jules-ch commented Sep 28, 2024

Seems to be done in #3324 (comment)

Can you add this in the list of the breaking changes in 5.1, it's not documented.
Also stubs from types-redis need to be changed`

@dmaier-redislabs
Copy link
Contributor

Hi Jules,

we had a note under bug fixes. I copied it over to the breaking changes section to make it more visible. Which RESP version are you using?

@vladvildanov I think we need to take a second look at this change.

Regards,
David

@jules-ch
Copy link
Author

I did not specified RESP protocol version somewhere, is it tied to Redis server version ?

@vladvildanov
Copy link
Collaborator

@jules-ch Thanks for reaching us! Changes in those PR should only affect RESP3 users, I'll have a look on it

@dmaier-redislabs
Copy link
Contributor

Hi @jules-ch ,

we did take another look and this impacts RESP2 and RESP3 with and without hiredis-py for response parsing. This was introduced as part of a bug fix in the context of dealing with RESP3 sets (message format).

So we decided that the original fix should never have impacted the result of the set (data structure) commands. We will release a patch version of 5.1 that reverts this breaking change for those commands. The original fix will still have an intended impact on some commands (e.g., the COMMAND command), but we will ensure that all the S* commands (e.g., SADD) will behave semantically correct again by returning a set instead of a list. We apologize for the inconvenience that this has caused.

Regards,
David

@jules-ch
Copy link
Author

jules-ch commented Oct 2, 2024

@dmaier-redislabs Alright makes sense, thanks for the quick fix, I'll wait before updating.

Closing this issue.

@jules-ch jules-ch closed this as completed Oct 2, 2024
@dmaier-redislabs
Copy link
Contributor

Reopening it until we released 5.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants