Add timeout to process codebase and plugin steps#5153
Add timeout to process codebase and plugin steps#5153AyanSinhaMahapatra wants to merge 1 commit into
Conversation
Signed-off-by: Ayan Sinha Mahapatra <asmahapatra@aboutcode.org>
|
Need some review there wrt. the default behaviour there, as these timeouts can cause weird issues/crashes elsewhere, so should we just keep this only as a CLI option which can be enabled to set a timeout, or have a default timeout like we have in the file-level scan timeout. If we have this as a default we also have to decide on a reasonable timeout duration. |
pombredanne
left a comment
pombredanne
left a comment
There was a problem hiding this comment.
Thanks! Could we do better and avoid another new option?
|
|
||
|
|
||
| DEFAULT_TIMEOUT = 120 # seconds | ||
| DEFAULT_PLUGIN_TIMEOUT = 2400 # seconds |
There was a problem hiding this comment.
This is way too much? 40 minutes feels forever. And why 40 minutes?
| f'[default: {DEFAULT_TIMEOUT} seconds]', | ||
| help_group=cliutils.CORE_GROUP, sort_order=10, cls=PluggableCommandLineOption) | ||
|
|
||
| @click.option('--timeout-plugins', |
There was a problem hiding this comment.
I find that new option confusing... and the default of 2400s super long
|
|
||
| # TODO: add progress indicator | ||
| # run the process codebase of each scan plugin (most often a no-op) | ||
| use_threading = processes >= 0 |
There was a problem hiding this comment.
Are you sure you want that? It seems not used at all
| type=float, | ||
| default=DEFAULT_PLUGIN_TIMEOUT, | ||
| metavar='<seconds>', | ||
| help='Stop an unfinished codebase processing or post-scan plugin after' |
There was a problem hiding this comment.
I cannot put my head around this... could we not use a simpler single timeout across the board instead?
2 participants
Reference:
Tasks
Run tests locally to check for errors.