Handle exception and kill restserver in nnictl #2086
Conversation
merge master
merge master
Update evolution doc (microsoft#1493)
merge master
merge master
merge master
augment pylintrc (microsoft#1643)
fix console.log (microsoft#1636)
merge master
merge master
merge master
merge master
Filter prune algo implementation (microsoft#1655)
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
Support monitor mode when creating or resuming a new experiment (microsoft#1933)
Add test for documentation build (microsoft#1924)
fix pipeline status badge (microsoft#1942)
merge master
merge master
merge master
merge master
| restServerPid = nni_config.get_config('restServerPid') | ||
| if restServerPid: | ||
| kill_command(restServerPid) | ||
| print_error(exception) |
There was a problem hiding this comment.
suggest to also add exit here
There was a problem hiding this comment.
This function is a top level function, no more code will be executed after this function. If there is error here, it will exit the process automatically.
There was a problem hiding this comment.
if there is an exception, the exit code should be non-zero.
if you catch here, even there is an exception, exit code is still 0.
There was a problem hiding this comment.
how about raising the error here instead of print_error?
There was a problem hiding this comment.
The output format of raising error is not elegant, NNICTL use uniform print_error() method for error information.
tools/nni_cmd/launcher.py
Outdated
| if restServerPid: | ||
| kill_command(restServerPid) | ||
| print_error(exception) | ||
| exit(1) | ||
| new_nni_config.set_config('restServerPort', args.port) |
There was a problem hiding this comment.
curious about this line. if users use --foreground, this line would not be executed, thus, new rest port would not be set, is this a problem?
There was a problem hiding this comment.
good point, moved new_nni_config.set_config('restServerPort', args.port) before launch_experiment() function.
No description provided.