[BE-170] Detect if there is no output and abort Step #801
+480
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lpusok
force-pushed
the
BE-83-detect-hangs
branch
from
fe13255 to
a07f884
Compare
lpusok
force-pushed
the
BE-83-detect-hangs
branch
from
2db8c39 to
6beebe9
Compare
lpusok
force-pushed
the
BE-83-detect-hangs
branch
from
ac0f703 to
d49743f
Compare
lpusok
changed the title
lpusok
force-pushed
the
BE-83-detect-hangs
branch
from
d4cb289 to
61d1252
Compare
ofalvai
reviewed
Aug 1, 2022
cli/run_config.go
Outdated
| } | ||
|
|
||
| func readNoOutputTimoutConfiguration(inventoryEnvironments []envmanModels.EnvironmentItemModel) time.Duration { | ||
| const defaultTimeout = 0 |
There was a problem hiding this comment.
I think it would be a bit safer to use -1 as the off value here instead of 0. A zero duration is something that other parts of the codebase can still interpret as a timeout and activate the hang detector accidentally.
ofalvai
reviewed
Aug 1, 2022
tools/timeoutcmd/timeoutcmd.go
Outdated
| c.timeout = timeout | ||
| } | ||
|
|
||
| // SetHangTimeout sets the timeout after which the command is killed when no output is received. |
There was a problem hiding this comment.
nit: no output is received on either stdout or stderr.
ofalvai
reviewed
Aug 1, 2022
tools/timeoutcmd/timeoutcmd.go
Outdated
|
|
||
| // SetHangTimeout sets the timeout after which the command is killed when no output is received. | ||
| func (c *Command) SetHangTimeout(timeout time.Duration) { | ||
| c.hangTimeout = timeout |
There was a problem hiding this comment.
Shouldn't this be inside the if too?
ofalvai
reviewed
Aug 1, 2022
ofalvai
reviewed
Aug 1, 2022
tools/hangdetector/hang_detector.go
Outdated
| outWriter io.Writer | ||
| } | ||
|
|
||
| func tickerSettings(timeout time.Duration) (interval time.Duration, tickLimit uint64) { |
There was a problem hiding this comment.
nit: this private function should go below the public ones in this file
ofalvai
approved these changes
Aug 1, 2022
lpusok
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The env
BITRISE_NO_OUTPUT_TIMEOUTcan be used to set a timeout in seconds, after which if there is no output from the current Step then it is aborted. The option is disabled by default.I plant to add in following PRs:
Solves: https://bitrise.atlassian.net/browse/BE-170