When kohsuke introduced TimeDuration in 2012, there were mistakes in the handling of time units. The naming of the internal field millis implies that it is measured in milliseconds, as does the design of the getTimeInMillis and as methods. Yet fromString treats 3sec as new TimeDuration(3), treating the field as measured in seconds.
Why did we not notice wildly incorrect scheduling times? Because the callers of this utility, currently ParametersDefinitionProperty and ParameterizedJobMixIn, also use it incorrectly. If the query parameter is set, they ask it for getTime (supposedly in milliseconds), yet pass it to scheduling methods which expect to be measured in seconds (as do quietPeriod fields). If it is not set, they construct it from quiet periods, passing in a value measured in seconds.
The parsing code was also intended to support other units, but never did.
The effect of all this mess is that .../build, .../build?delay=3sec, and .../build?delay=3 all behave as you expect, so there is not really a user-visible bug, but the code is very confusing.
Since TimeDuration is not Serializable, there is no stored-settings issue here, so it would suffice to change <init>(long) to * 1000 for compatibility but deprecate it in favor of a constructor taking an explicit unit; change getTime to / 1000 and deprecate in favor of as; and change all call sites to use nondeprecated forms, passing in TimeUnit.SECONDS.
- causes
-
JENKINS-56542 Build Token Root delays 1000 times longer than expected
- Resolved
- relates to
-
JENKINS-48770 Quiet period is in milliseconds
- Resolved
- links to