Easier extendablity with custom commands. #133
Conversation
This adds customisability via subclassing to the Monitor. The standard library module Cmd was a loose reference point when it comes to the interface. + small fixes here and there.
Codecov Report
@@ Coverage Diff @@
## master #133 +/- ##
========================================
- Coverage 92.82% 92% -0.83%
========================================
Files 4 4
Lines 251 300 +49
Branches 32 39 +7
========================================
+ Hits 233 276 +43
- Misses 11 16 +5
- Partials 7 8 +1
Continue to review full report at Codecov.
|
|
You mean https://docs.python.org/3/library/cmd.html this one? I did not know that such thing even exists :) |
|
Sorry for delay, I will PR more closely in few days. |
| @@ -94,20 +108,29 @@ def test_basic_monitor(monitor, tn_client, loop): | |||
| assert 'Task' in resp | |||
|
|
|||
| resp = execute(tn, 'ps 123\n') | |||
| assert 'Task' in resp | |||
| assert 'TypeError' in resp | |||
There was a problem hiding this comment.
Can you elaborate on this one? Why this change?
There was a problem hiding this comment.
Because the new code for looking up the method to execute will take the method (do_ps in this example) and give it any arguments supplied by the user (123). The TypeError is simply the error that Python raises when you give an incorrect amount of arguments to a function.
I am currently working on implementing more of the public interface of Cmd where this will picked up.
|
|
||
| resp = execute(tn, 'signal name\n') | ||
| assert 'Unknown signal name' in resp | ||
|
|
||
| resp = execute(tn, 'stacktrace\n') | ||
| assert 'loop.run_forever()' in resp | ||
|
|
||
| resp = execute(tn, 'wehere 123\n') |
There was a problem hiding this comment.
Why it worked in the past you mean? Because with the old code any command beginning with a "w" would be routed to "where".
There was a problem hiding this comment.
yeah, stupid error on my side :)
There was a problem hiding this comment.
It did take me a while to realise what was going on as well :D
|
@jettify Yes, that's the standard library cmd module. I thought it useful to try and adhere to its API while making aiomonitor more extensible. If you have any comments about the direction this is taking, I'm listening :) Building on this PR I'm working on adding more of the Cmd-API (like |
|
I like your idea and implementation (thanks for updating docs and example!), give me few days to reread PR more carefully. |
This adds the ability for the user to add custom commands to a Monitor subclass, inspired by the standard library module "Cmd".