Skip to content

Makes jsonrpc_handler more flexible#92

Merged
RobRuana merged 4 commits intomasterfrom
improves_jsonrpc_services
Nov 18, 2017
Merged

Makes jsonrpc_handler more flexible#92
RobRuana merged 4 commits intomasterfrom
improves_jsonrpc_services

Conversation

@RobRuana
Copy link
Copy Markdown
Contributor

@RobRuana RobRuana commented Nov 14, 2017

The jsonrpc_handler was created with the intent that it would be added as a member of a class, so it was given a single positional parameter named self. This unfortunately breaks when the jsonrpc_handler is used outside the context of a class (it throws "missing required self argument" errors).

The self parameter is never actually referenced. This pull request switches the parameter from self to self=None, so the jsonrpc_handler function can be invoked outside the context of a class.

@EliAndrewC
Copy link
Copy Markdown
Contributor

+1

It looks like the Travis tests are failing for this branch but passed the last time they were run several months ago. However, it looks like they exit entirely as soon as the server tests begin, which leads me to believe that the problem isn't with this code but rather because Travis has perhaps begin to be more strict about whether we're allowed to do stuff like have a socket open a TCP port and make connections to it.

This leads me to believe that we could get our tests running again by changing this line
https://github.com/magfest/sideboard/blob/master/tox.ini#L32
to have -m 'not functional' and thus omit the "functional" tests which actually start and stop CherryPy from being run by Travis. This isn't ideal, but would probably be a good thing to try in the meantime.

If you feel like making and testing that change as part of this PR so that merging this doesn't break Travis then that would be great. If you don't want to hold up this PR since we have other stuff which depends on it, then I'd be okay with this getting merged in as-is and punting on getting Travis running again.

@RobRuana RobRuana merged commit 88469b9 into master Nov 18, 2017
@RobRuana RobRuana deleted the improves_jsonrpc_services branch November 18, 2017 19:35
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