Until we have some kind of remote logging we need not to catch remote
execution exceptions and stop the execution so we know something went
wrong. Also, if remote execution fails, we don't try to get logs from
minion since there is a chance that the minion is dead and we'd hang
while trying to get the logs. If there is any error during remote
execution AND during teardown, raise the remote execution exception
and just log the teardown one since the former is more important to
us.
Details
- Reviewers
lbrabec kparal tflink jskladan - Maniphest Tasks
- T628: errors during remote execution are not propagated up
- Commits
- rLTRN30147c40f9ef: Propagate errors during remote execution
Ran tasks locally both in --ssh and --libvirt, seemed to worked. Tests pass as well.
Diff Detail
- Repository
- rLTRN libtaskotron
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Tiny nit but otherwise LGTM
libtaskotron/minion.py | ||
---|---|---|
142 | tiny nit but I'd like to see a longer var name here, took me a sec to figure out what the var was for |
libtaskotron/minion.py | ||
---|---|---|
142 | If that name is confusing (and it's missing p;) ), I'd use something clearer like execution_ok. False by default, True at the end of the try block. Then we can leave out the except block completely (which also removed an edge case when a different exception is thrown and teardown fails as well). |
libtaskotron/minion.py | ||
---|---|---|
174 | Maybe I would add something like: # a different exception is already being raised, don't override it |
tiny nit but I'd like to see a longer var name here, took me a sec to figure out what the var was for