-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tag to disable random sleep upon a renew task #6599
Tag to disable random sleep upon a renew task #6599
Conversation
…vely a certificate to renew.
Yes, I was charmed by Pycharm... and put more code cleaning that code logic. I will remove that, and make some PRs with only code cleaning, no changes of logic. |
So currently the logic in this PR is in sync with the latest comment of @bmw in #6596. However there is still an open discussion about this. Here as the options so far:
I also expressed another proposition on IRC. Because personally, I am more concerned about the hard coded time range than about enabling or not the sleep. Indeed I think now that the sleep should not be disabled, it is a good thing to dispatch load on ACME CA. However, I think that having a fixed time range, that will block a process for at most this time range "for no reason", and out of the context, is a weird approach. What about the time range value, 8 min? Why not 10 min, 1 h or even 24 h to be sure to dispatch other the day? So I would prefer to allow to modify the value of the range. Preferably as an hidden flag, as it is still more an option for certbot developers and integrators. |
All right @ohemorange. All changes are implemented, ready for new review. |
Thanks for making the changes! Also, ooh, a shiny new tool that looks like it automatically PEP8ifies things...... maybe I'll try pycharm... As for time range vs disable: The two uses of this flag that are the reason we're putting it in are:
For both of these, completely disabling it solves the problem. So in the spirit of YAGNI, I'm in favor of keeping it as is. |
FYI, after trying sublime text, atom, eclipse, visual studio code, I found out PyCharm to be the most advanced and efficient IDE I ever used for Python. But it makes non PEP8 compliant code extremely etchy ^^ |
Following #6596, I extracted out my existing solution from #6541.
It adds a flag to the CLI,
--no-random-sleep-on-renew
to disable the random sleep when renew action is called from non interactive shells. The default behavior stays to make the random sleep if the flag is not provided.The random sleep logic is also moved on layer down, to avoid to trigger it when no renew is going to be executed. Of course, this logic will be executed only once for a set of certificates to be renewed.