(maint) Deprecate fine grained control of file and environment timeouts#7001
(maint) Deprecate fine grained control of file and environment timeouts#7001justinstoller wants to merge 1 commit intopuppetlabs:masterfrom
Conversation
|
CLA signed by all contributors. |
Magisus
left a comment
Magisus
left a comment
There was a problem hiding this comment.
I think using the hooks makes sense. Or are you worried that the value will have already been munged by the time it gets to the hook? (in which case I'll try it)
| releases this value will only determine if file content is cached. | ||
|
|
||
| Valid values will be 0 (never cache) and 'unlimited' (always cache). | ||
| With a default of 'unlimited'. |
There was a problem hiding this comment.
Seems a little odd that the default is not one of these values...
There was a problem hiding this comment.
Yeah, 'unlimited' is valid for TTL settings but not for Durations.
Currently I believe users work around this by setting this to some absurdly large number for a timeout like 5y.
I figured moving it to a TTL type was out of scope. I could add comment that unlimited, while not valid now, will be the default and that users who have this set to a really large value now should keep the move in mind?
There was a problem hiding this comment.
I guess reading it again the current comment does imply that it will work that way in the future. I think this is fine.
|
Yeah, 0 and 'unlimited' or '15s' were the values I saw when trying to test this out (but I might not have been initializing Puppet correctly??). I found that concerning because I would have expected the Numerics |
justinstoller
force-pushed
the
deprecate-fine-grained-config-reloading
branch
from
409b284 to
2dfdf82
Compare
|
Re-targeted at master, I think this is still a Good Thing™ to get into Puppet 6. |
|
hey @puppetlabs/platform-core, since this was a proposed deprecation for 6, it would be nice to get a look at getting this in today |
|
FWIW, this isn't a blocker for 6. |
|
For sure, just thought I'd help bring it back up since the pr has been sitting for a while and doesn't seem to have any blockers itself :) |
|
We should probably retarget this at We'll deprecate this in 6, and remove in 7. @joshcooper will file the companion ticket for puppet 7. |
|
Puppet 7 ticket added in https://tickets.puppetlabs.com/browse/PUP-9262 |
|
Ping @justinstoller the PR is targeting master, but @melissa last comment suggested targeting 6.0.x. But 6.1.0 is going out soon. What would you like to do? |
|
Sorry, not really on my radar any more. I'm fine with wherever as long as we get it in for 7 (or whenever we pick up the file-serving re-write if sooner). |
|
I think this is something, if we still want it, we should get into 6.y. How long do we have to let deprecations like this soak before we remove them? If we get the deprecation into 6.y, can we still remove it in 7? I opened https://tickets.puppetlabs.com/browse/PUP-9497 for the deprecation, so that we can add and publish release notes about the deprecation. |
|
@puppetlabs/puppetserver-maintainers I assume y'all still want this change? If so, I'll take it, rebase it onto 6.0.x, and update the commit to reference the ticket. |
|
@justinstoller I opened #7433 which references https://tickets.puppetlabs.com/browse/PUP-9497 in the commit and targets the 6.0.x branch. I'm closing this PR in favor of that one |
6 participants
This is a nice to have deprecation regardless of whether we ever implement file serving natively in Puppet Server. See SERVER-2285 and SERVER-2028 for background on these two settings (and how they don't work well currently).
I'm a little unsure on the calling of
:hookto ensure the values I'm checking against haven't been munged yet.