adding status message for bad request + tests #1022
Conversation
|
@justinvdm not quite sure why the test is failing? |
|
The failures might be related to treq no longer supporting Python 2.6. I
|
…at-transport-integration
|
@hodgestar @justinvdm ready for review :) |
| message['message_id'], | ||
| reason='Received status code: %s' % (response.code,)) | ||
| returnValue(nack) |
There was a problem hiding this comment.
Is there a reason this method was converted to inlineCallbacks?
There was a problem hiding this comment.
@justinvdm knows more about the reasoning I think
There was a problem hiding this comment.
Can't remember tbh, maybe because we were doing other things in this method at some stage (like publishing statuses). I think we can put it back to how it was.
There was a problem hiding this comment.
|
@hodgestar @justinvdm added the good request stuff |
| request.setResponseCode(http.BAD_REQUEST) | ||
| return '' | ||
| raise WeChatException('Bad request for incoming message') | ||
| elif is_verifiable(request): |
There was a problem hiding this comment.
I don't think we need to check is_verifiable again here. If we don't raise the exception, we should always call add_status_good_req?
| 'signature': good_signature, | ||
| 'timestamp': timestamp, | ||
| 'nonce': nonce, | ||
| } | ||
|
|
||
| default_params.update(params) | ||
| path += '?%s' % (urlencode(default_params),) | ||
| params.update(underrides) |
There was a problem hiding this comment.
The params handling is a bit odd. The underrides are possibly what is passed in by the user (i.e. underrides = kw.setdefault('params', {})) and they override the default params (i.e. params.update(underrides)). Maybe it would be clearer to do:
params = {
'signature': good_signature,
...
}
params.update(kw.get('params', {}))
kw['params'] = params?
|
@codiebeulaine Please no commit messages like |
|
Left a couple of comments and questions, but otherwise looks good. :) |
|
@hodgestar changed some things above, Do I need to change the method above back or can I get a +1 ? |
|
👍 |
…gration adding status message for bad request + tests
No description provided.