Skip to content

Commit

Permalink
config: Require strictly-positive timeout values
Browse files Browse the repository at this point in the history
If the timeout value was zero, the hook would always error, and there
doesn't seem to be much point to that.  And I'm not sure what negative
timeouts would mean.  By adding this restriction, we do not limit
useful hook entries, and we give the validation code grounds to warn
users attempting to validate configs which are poorly defined or have
useless hook entries.

I'd like to remove the pointer from the Go type to comply with our
anti-pointer zero-value style (style.md) now that Go's zero-value is
clearly invalid.  However, there has been maintainer resistance to
removing the pointer [1] (although I don't think this is consistent
with previous maintainer statements that we don't need pointers when
the zero value is invalid [2]).  In order to land the normative spec
change, this commit keeps the current *int for Hook.Timeout and punts
a consistent policy to future work.

[1]: #764 (comment)
[2]: #653 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
  • Loading branch information
wking committed Apr 15, 2017
1 parent 0946333 commit ecf7314
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 1 deletion.
1 change: 1 addition & 0 deletions config.md
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ Hooks allow for the configuration of custom actions related to the [lifecycle](r
* **`args`** (array of strings, OPTIONAL) with the same semantics as [IEEE Std 1003.1-2001 `execv`'s *argv*][ieee-1003.1-2001-xsh-exec].
* **`env`** (array of strings, OPTIONAL) with the same semantics as [IEEE Std 1003.1-2001's `environ`][ieee-1003.1-2001-xbd-c8.1].
* **`timeout`** (int, OPTIONAL) is the number of seconds before aborting the hook.
If set, `timeout` MUST be greater than zero.
* **`poststart`** (array of objects, OPTIONAL) is an array of [post-start hooks](#poststart).
Entries in the array have the same schema as pre-start entries.
* **`poststop`** (array of objects, OPTIONAL) is an array of [post-stop hooks](#poststop).
Expand Down
3 changes: 2 additions & 1 deletion schema/defs.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@
"$ref": "#/definitions/Env"
},
"timeout": {
"type": "integer"
"type": "integer",
"minimum": 1
}
},
"required": [
Expand Down

0 comments on commit ecf7314

Please sign in to comment.