WIP: Start on a set of common callbacks #163
Conversation
Codecov Report
@@ Coverage Diff @@
## master #163 +/- ##
==========================================
- Coverage 95.68% 95.16% -0.53%
==========================================
Files 13 15 +2
Lines 1343 1467 +124
Branches 166 197 +31
==========================================
+ Hits 1285 1396 +111
- Misses 43 56 +13
Partials 15 15
Continue to review full report at Codecov.
|
Signed-off-by: Jeremy Cline <[email protected]>
Runs a configurable command when a message is received. Fixes fedora-infra#155 Signed-off-by: Jeremy Cline <[email protected]>
cverna
force-pushed
the
common-callbacks
branch
from
723217c to
a2be788
Compare
|
I think we should go with that for now, and based on the usage ask people to file RFEs. I have added a simple test case for the runner callback. Do we need anything else to merge this PR ? |
This commit add a simple test case for the runner callback. It also disable the bandit security checks against the subprocess call since the choice to use shell=True is left to the user of the callback. Signed-off-by: Clement Verna <[email protected]>
cverna
force-pushed
the
common-callbacks
branch
from
a2be788 to
821bdc1
Compare
| processing. Defaults to no timeout. | ||
| """ | ||
| try: | ||
| subprocess.check_call( # nosec |
There was a problem hiding this comment.
I think we need @puiterwijk 's approval to add the nosec tag. Can you follow up with him @cverna ?
| with mock.patch("subprocess.check_call") as mock_subprocess: | ||
| callbacks.run(message) | ||
|
|
||
| mock_subprocess.assert_called_once_with("/bin/true", shell=False, timeout=None) |
There was a problem hiding this comment.
Could you also check that the Drop exception is raised when the command fails, and that the proper exception is raised when the command times out?
|
Seems likely I'm not going to finish this. |
Move the printer callback into the "callbacks" module and add the callback requested in #155.
There are no tests yet because I wanted to get a bit of feedback on usage. @nirik, would the command ever need to see the message itself, or is this purely a way to trigger a command based on the topic alone? Would it ever be helpful to requeue the message and run the command again if it gets stuck while running or exits with some non-zero return code?
The current configuration options are:
command(string or list of strings): The arguments to launch the processas a string or list of strings.
shell(boolean): Whether or not to execute the command in a shell. Refer to thesecurity considerations <https://docs.python.org/3/library/subprocess.html#security-considerations>_before setting this to true. Defaults to false.
timeout(integer): The length of time, in seconds, to wait on the commandbefore halting the command and returning the message to the queue for later
processing. Defaults to no timeout.
Would anything else be useful to configure here?