controller: replaced parse_duration package with custom parser

[mjsterckx]

Issue number:

Closes #596

Description of changes:

The parse_duration package is no longer being maintained. Since we used the package for a very specific use-case (parsing the timeout string, which is already validated by regex), it was easier to remove the package and implement a short function to replace it. This function takes a string that conforms to "^((([0-9]+)d)?(([0-9]+)h)?(([0-9]+)m)?(([0-9]+)s)?|\d+)$" and converts it into a Duration object.

Testing done:

Created Test object with various valid values for timeout ("123", "1d2h3m4s", "500s", "3h100s", "20d", etc.) which all resulted in the correct Duration. When a value was used for timeout that didn't conform to the regex, the object could not be created (as is expected).

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

[webern]

Nice. Please add unit tests.

[mjsterckx]

Since the new function is only called with values that are previously checked with regex, running the function with a value that doesn't conform to the regex is undefined behavior. Should I "define" this behavior and add some unit tests for those cases, or only add tests for values that conform to the regex?

[webern]

Since the new function is only called with values that are previously checked with regex, running the function with a value that doesn't conform to the regex is undefined behavior. Should I "define" this behavior and add some unit tests for those cases, or only add tests for values that conform to the regex?

It's not super important in this case, so not worth losing a bunch of time on, but...

It probably wouldn't hurt to use ? instead of unwrap_or_else([default]). If you use anyhow::Error (via crate::Error) instead of ParseIntError then I think it's pretty easy to define the behavior of this function such that it errors on unexpected input. And it's always good to have unit tests that cover these "bad input" cases.

[ecpullen]

This should return an error type from this crate.

[mjsterckx]

^ Added unit tests and removed parse_duration and its dependencies from Cargo.lock

[webern]

I don't quite follow the parsing logic, but the tests convince me that it works.

[stmcginnis]

Looks like unit tests cover each case, including invalid cases. Might be nice to have the error message contain something more detailed as to why the parsing failed, but hopefully that should be pretty obvious to future us.

[etungsten]

Not sure if the parsing function belongs in the constants module. Maybe a new utils.rs is more appropriate.