Correctly handle Resources #30
Conversation
| @@ -25,6 +25,12 @@ def ensure_utf8_bytes(v): | |||
| return v | |||
|
|
|||
|
|
|||
| class StandInResource(object): | |||
There was a problem hiding this comment.
Noted, changed in the next patch.
|
@dreid Incorporated your review comments! |
|
|
||
|
|
||
| class MockProducer(object): | ||
|
|
There was a problem hiding this comment.
I don't generally put an empty line here unless it's between a docstring and the first method definition.
|
@dreid okay, gone through and fixed up those things. Unfortunately it looks like I'll have to use a reactor, since you're right about task.Clock not doing what I intended to do. |
|
|
||
| def resumeProducing(self): | ||
| self.count += 1 | ||
| if self.count <= 3: |
There was a problem hiding this comment.
What's special about the number 3? Can we make the number a param passed in through ProducingResource.__init__ and MockProducer.__init__ so that the number 3 is closer to the assertion of it being called 3 times?
There was a problem hiding this comment.
It's not 1! :) It's just a number I picked from random, really.
|
Unless anyone else has any other comments, I think this is good to go. |
| producer.resumeProducing() | ||
| producer.resumeProducing() | ||
| request.producer = producer | ||
| for x in xrange(1,3): |
There was a problem hiding this comment.
xrange is for supporting ancient versions of Python and large ranges. range(2) is what you want here, I think.
|
Thanks @exarkun , I've made those changes. |
| @@ -102,18 +111,20 @@ def _execute(): | |||
| d = defer.maybeDeferred(_execute) | |||
|
|
|||
| def write_response(r): | |||
| if isinstance(r, unicode): | |||
| r = r.encode('utf-8') | |||
| if not r is _StandInResource: | |||
There was a problem hiding this comment.
This would typically be written as:
if r is not _StandInResource:
|
Thanks @dreid , fixed those two things. |
| producer.resumeProducing() | ||
| producer.resumeProducing() | ||
| request.producer = producer | ||
| for x in range(1,3): |
There was a problem hiding this comment.
you really want just range(2) here as jp wrote. :)
This fixes issue #28.