Skip to content

Conversation

ChuckHend
Copy link
Contributor

Decode message_id to str when it is provided as bytes.

Closes #17

@ChuckHend ChuckHend marked this pull request as ready for review March 24, 2025 14:17
@ChuckHend
Copy link
Contributor Author

@mlasevich ☝️ 😃

@mlasevich
Copy link
Owner

My apologies, will take a look

Copy link
Owner

@mlasevich mlasevich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It pays for me to replicate the issue locally to understand it better. Now that I look at it. its pretty bad. Thanks for the contribution.

if int(result[0]) == 1 and int(result[1]) > 0:
# 1 key deleted from zset
# 3 keys deleted from hash (message itself, receive count, first receive field)
if int(result[0]) == 1 and int(result[1]) == 3:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a scenario where result[1] > 0 but not 3?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, unless the number of keys stored per message in the hash changes. From what I could tell, the message, receive count, and first receive are always there, so if there are not deleted then there would be some sort of problem. That was my thought process around this.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense.... BTW, it is out as v0.6.1 🤞🏻


# decode to string when provided as bytes
if isinstance(message_id, bytes):
message_id = message_id.decode('utf-8')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to avoid issues with tx.zrem() we may want to adjust the message id specifically for generated rc/fr keys - but it seems to work fine as-is

@mlasevich mlasevich merged commit d7c8314 into mlasevich:master Apr 16, 2025
1 of 2 checks passed
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.

incomplete delete when using message_id: bytes
2 participants