refactor: refine the Handler validation #870
Conversation
pjenvey
added this to the 2017Q2-2.2 Four code refactor issues completed and obsolete legacy code removed... milestone
Codecov Report
@@ Coverage Diff @@
## master #870 +/- ##
======================================
Coverage 100% 100%
======================================
Files 50 50
Lines 9570 9883 +313
======================================
+ Hits 9570 9883 +313
Continue to review full report at Codecov.
|
| def _make_test_data(self, headers=None, body="", path_args=None, | ||
| path_kwargs=None, arguments=None): | ||
| def _make_test_data(self, headers=None, body="", args=None, kwargs=None, | ||
| arguments=None): |
There was a problem hiding this comment.
Hrm. While it feels like a nit, args vs. arguments might be a bit confusing. Granted, path_args wasn't terribly great either, but at least it reflected what it was used for.
Since it's kinda/sorta unused. I suppose we could just drop the args for now.
There was a problem hiding this comment.
I had the same thought and forgot all about it, I was thinking of renaming arguments (how about request_arguments?) it's rarely used anyway
There was a problem hiding this comment.
eh, maybe still a bit confusing..
There was a problem hiding this comment.
Yeah, like I said, a bit more nitty than needed. I'm also fine with just dropping "args" since I don't think that anything actually passes them. Calling "arguments" request_arguments might be good. Granted, shortening down 'kwargs' and tossing it into the middle of the arg list might cause a few python folk to twitch, but none of that is super critical.
There was a problem hiding this comment.
Punting on this for now, I'll revisit it later. cyclone apparently sets up path_args/kwargs for use by the prepare method, so I guess there's a small case to keep the naming, but I wouldn't mind shortening. the websocket handlers still use regular args (though they don't validate), should probably switch those over
autopush/web/registration.py
Outdated
| @@ -133,6 +137,41 @@ def validate_data(self, data): | |||
| errno=109, | |||
| headers=request_pref_header) | |||
|
|
|||
| @post_load | |||
| def handler_kwargs(self, data): | |||
| # not used | |||
There was a problem hiding this comment.
this just pops it off the handler kwargs after it's validated (not used = not used by the handlers)
autopush/tests/test_endpoint.py
Outdated
| "uaid": uaid, | ||
| "chid": chid} | ||
| return dict() | ||
| def _req(self, m, router_type="", router_token="", uaid=None, chid=None): |
| class RouterInfo(object): | ||
| """Bundle of Router registration information""" | ||
|
|
||
| router = attrib() # type: Any |
There was a problem hiding this comment.
It's really Any? I thought there were only a few valid sets of things it could be.
There was a problem hiding this comment.
It's IRouter, however, mypy doesn't yet understand zope.interface
- instead of valid_input call handler methods w/ the resulting dict as args/kwargs (similarly to how cyclone does) - bundle router info together in registration to ease passing it around - kill the now unneeded MessageSchema args/kwargs shuffle and its associated test_delete_topic_success2 - fix LogCheckSchema typo closes #695
closes #695