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

Timeout support for list keys and buckets #100

Merged
merged 3 commits into from
May 24, 2013
Merged

Conversation

evanmcc
Copy link
Contributor

@evanmcc evanmcc commented Apr 18, 2013

add:

  • streaming list buckets
  • timeouts for list keys
  • timeouts for list buckets

@@ -1100,14 +1129,24 @@ process_response(#request{msg = #rpbdelreq{}},

Choose a reason for hiding this comment

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

This is subjective so feel free to completely ignore this comment, but I think this function is too long and would be easier to understand if refactored to use a couple of function calls. e.g. maybe_send_caller and process_response_reply. Another option I would like better would be to use a few function clauses to replace the case statements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is copied directly from the key-listing clause below it. I really with that erlang allowed conjunctions in it's pattern-matching, it'd simplify a lot of code. But the, I wish a lot of things about erlang. I'll see if I can make both of them a bit cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I am just going to clean up mine, since the old one seems to work, and I don't know the history/backwards compatibility requirements. But since there is no history for mine I can guarantee that buckets and done will never be set in the same response, so can simplify it down to two clauses.

Choose a reason for hiding this comment

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

I like that a lot better.

@evanmcc
Copy link
Contributor Author

evanmcc commented May 24, 2013

updated.

@kellymclaughlin
Copy link

+1 to merge

@ghost ghost assigned evanmcc May 24, 2013
evanmcc added a commit that referenced this pull request May 24, 2013
Timeout support for list keys and buckets
@evanmcc evanmcc merged commit 81d0a34 into master May 24, 2013
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.

2 participants