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

Fix bug that prevented messages via patternchannel from getting sent #11

Merged
merged 3 commits into from Oct 9, 2019

Conversation

@jacobstoehr
Copy link
Contributor

commented Oct 8, 2019

This bug occured when a user sent a message to a channel that was pattern
subscribed to. Then, the _handle_get_command function would return None as the
channel that was requested was not in the allowed channel names.
This was fixed by adding a check that returned True if the channel_name appeared
in the channel_patterns, thus passing that first test in _handle_get_command.

@jacobstoehr jacobstoehr requested a review from zvyn Oct 8, 2019
Copy link
Contributor

left a comment

Looks good, just a not missing and cosmetic suggestions

EDIT: Oh and please rebase to master, I added GitHub Actions testing in Python 3.5 and Python 3.6 to verify compatibility.

redis_websocket_api/handler.py Outdated Show resolved Hide resolved
redis_websocket_api/protocol.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
This bug occured when a user sent a message to a channel that was pattern
subscribed to. Then, the _handle_get_command function would return None as the
channel that was requested was not in the allowed channel names.
This was fixed by adding a check that returned True if the channel_name appeared
in the channel_patterns, thus passing that first test in _handle_get_command
@jacobstoehr jacobstoehr force-pushed the fix_pattern_channel_no_message_returned branch from 8350234 to ba2c9b5 Oct 9, 2019
This led to the discovery of a bug in this bugfix. It has been fixed by
discarding the last char of channel_patterns (always a literal `*`).
Now the _channels_in_patterns function actually returns True if it matches. This
was not discovered before because of the missing `not` statement.
to reflect the change in the signature of the handler __init__ function.
@jacobstoehr jacobstoehr force-pushed the fix_pattern_channel_no_message_returned branch from ba2c9b5 to b84dc92 Oct 9, 2019
@jacobstoehr

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2019

I have implemented your suggestions. In the process of doing so, I had to update the tests because I added a new argument to the init function of the Handler class.
Please review again. Thank you!

@zvyn
zvyn approved these changes Oct 9, 2019
@zvyn zvyn merged commit 0e24ec6 into master Oct 9, 2019
4 checks passed
4 checks passed
build (3.5)
Details
build (3.6)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@zvyn zvyn deleted the fix_pattern_channel_no_message_returned branch Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.