Allow other time units in notification and scheduleddowntime #220
Conversation
| @@ -123,7 +123,7 @@ | |||
| if !is_string($user_groups) { validate_array($user_groups) } | |||
| if $times { validate_hash ($times )} | |||
| if $command { validate_string ($command )} | |||
| if $interval { validate_integer ($interval )} | |||
| if $interval { validate_re($interval, '^\d+\.?\d*[d|h|m|s]?$')} | |||
There was a problem hiding this comment.
Please use: \d+(\.\d+)?[dhms]?
Your syntax is not really correct, its either (d|h|m|s) or [dhms]
There was a problem hiding this comment.
Okay, I copied it from https://github.com/Icinga/puppet-icinga2/blob/master/manifests/object/service.pp#L216 without thinking,
| @@ -88,7 +88,7 @@ | |||
| if $author { validate_string($author) } | |||
| if $comment { validate_string($comment) } | |||
| if $fixed { validate_bool($fixed) } | |||
| if $duration { validate_integer($duration) } | |||
| if $duration { validate_re($duration, '^\d+\.?\d*[d|h|m|s]?$') } | |||
There was a problem hiding this comment.
change the regexp here too
jkroepke
force-pushed
the
dependency_duration
branch
from
1dbe9c3 to
27ca964
Compare
|
I guess we should update that everywhere. Want to do it? Would you have a look on the failing tests, or should we? |
Can I look into it. |
|
All breakage relevant tests failed, because the error message has changed. I wound why there are test for breakage since you comment long time okay that is usually or unnecessary. |
|
Noticed that myself while activating travis tests. Will discuss that this week with the guys. Had similar problems with expecting for string behavior between Puppet 3+4. Could you update the tests so:
Appreciate your help here ;) |
jkroepke
force-pushed
the
dependency_duration
branch
from
27ca964 to
29ed275
Compare
|
Hi @jkroepke Please let me know if you will look into other objects, if they need this kind of updates. I just want to have a plan how we will proceed with this. |
|
I fixed the specs and merged into master |
Hi,
while i'm moving from the old puppet to the new, I have notice that values like 30m are not allowed anymore in some parameters.
Here is a PR for 2 cases, maybe there are a lot of more...