Support fractional keep_jobs times

[squidpickles]

What does this PR do?

Adds support for fractional keep_jobs interval times, so that times less than an hour can be specified.

What issues does this PR fix or reference?

See #55295

Previous Behavior

Times specified are converted to ints.

New Behavior

The time is now a float, so smaller / more precise intervals can be specified. Still using hours, rather than integer minutes or seconds, to preserve backwards compatibility.

Tests written?

Existing tests apply appear to pass

Commits signed with GPG?

Yes

[dwoz]

@squidpickles If we're going down this path, I think it'd make more sense to define keep_jobs in seconds. This would however require a depreciation path but could be implemented behind a feature flag.

[squidpickles]

@dwoz Yes, I had that thought, but wasn't sure how much backwards compatibility factored in. If you can describe how this project makes breaking changes, I would be happy to implement it.

[twangboy]

This needs a testcase and a changelog. This will also need to be rebased and conflicts fixed.

[Ch3LL]

ping @squidpickles this is the documentation on how to deprecate code: https://docs.saltproject.io/en/latest/topics/development/deprecations.html

Are you willing to come back to this PR and fix up the merge conflicts and resolve the requested feedback?

[squidpickles]

ping @squidpickles this is the documentation on how to deprecate code: https://docs.saltproject.io/en/latest/topics/development/deprecations.html

Are you willing to come back to this PR and fix up the merge conflicts and resolve the requested feedback?

Ah, that is helpful. Yes, I can do this. It may take me a few days to get to it, but happy to use integer seconds with a new field name.

[squidpickles]

Not sure about all the tests failing above. Is that common? I can't see any obvious errors in the (truncated) log output

[Ch3LL]

It looks like the job failures are related to this change. Its getting a keyerror in one of the tests because its expecting keep_jobs to exist.

I've also asked for @dwoz feedback on this PR

[dwoz]

Approved, assuming we fix the tests.

[Ch3LL]

Just a small request. Can you change the filename to .deprecated. We have not changed anything yet, because you can still use keep_jobs so the describption should reflect that keep_jobs is on a deprecation path and will be removed in favor of keep_jobs_seconds in the Chlorine release.

[Ch3LL]

I went ahead and change the filename and I updated the release to Argon. I was wrong in recommending Chlorine as we need to ensure the deprecation is in two releases before removing

[squidpickles]

I went ahead and change the filename and I updated the release to Argon. I was wrong in recommending Chlorine as we need to ensure the deprecation is in two releases before removing

Thanks for that. Makes sense.

Not sure about tests passing or not -- I have fixed the KeyError at any rate

[welcome]

Congratulations on your first PR being merged! 🎉