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

Nested multi bulk 2011 06 18 #32

Closed
wants to merge 3 commits into from
Closed

Nested multi bulk 2011 06 18 #32

wants to merge 3 commits into from

Conversation

jdavisp3
Copy link
Contributor

Accept nested multi-bulk responses. These can be returned from
execute() commands where the transaction includes commands that
normally return multi-bulk responses.

@@ -114,8 +114,7 @@ class RedisBase(protocol.Protocol, policies.TimeoutMixin, object):
self._buffer = ''
self._bulk_length = None
self._disconnected = False
self._multi_bulk_length = None
self._multi_bulk_reply = []
self._multi_bulk_stack = [] # [[length-remaining, [replies] | None]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Making self._multi_bulk_stack a collections.deque might be a better idea since you're right-popping it a lot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I will make the change.

@rlotun
Copy link
Collaborator

rlotun commented Jul 8, 2011

I think other than this (debatable) data structure change, we're good to go!

@jdavisp3 jdavisp3 closed this Jul 9, 2011
@jdavisp3
Copy link
Contributor Author

jdavisp3 commented Jul 9, 2011

Closing this request as I just sent a new replacement.

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.

None yet

2 participants