Skip to content

Fix(storage): correct pipeline usage of RedisStorage to retrieve event_id and prevent type errors #177

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Jimmy-web169
Copy link

Description

What did this PR change

  • Refactored the Redis pipeline usage to execute INCR before using its result as event_id.
  • Changed from single pipeline chain to a two-step execution:
    • First pipeline: INCR command to increment event counter and retrieve the new event_id.
    • Second pipeline: SETEX with the correctly retrieved event_id.

Why was this change necessary

The original implementation misused the Redis pipeline API by directly assigning the result of pipe.incr(...) to event_id.

However, in Redis-py, calling pipe.incr(...) does not immediately execute and instead queues the command, returning the pipeline object itself. This led to:

  • event_id being a Pipeline object, not an integer.
  • Consequently, building the Redis key resulted in:
"event:my_channel:<redis.client.Pipeline object at 0x7f8b8c0d5f40>"

-This also causing the following error when running django setting with "django_eventstream.storage.RedisStorage":

TypeError: unsupported operand type(s) for -: 'Pipeline' and 'int'

Notes

  • File changed: django_eventstream/storage.py
  • This is currently implemented as a workaround, splitting the pipeline into two executions to ensure we have a concrete event_id before using it in the second operation.
  • Future improvements might include to keep it atomic in a single transaction if needed.

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

Successfully merging this pull request may close these issues.

1 participant