📖 Improve docs for manager.Options.SyncPeriod #1232
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/S
The comment tries to capture more of the thoughts exchanged in kubernetes-sigs#88.
dlipovetsky
force-pushed
the
explain-syncperiod
branch
from
463b1da
to
baf3d61
Compare
|
(Updated for clarity and force-pushed) |
| // | ||
| // If you want | ||
| // 1. to insure against missed watch events, or | ||
| // 2. to poll services that cannot be watched, |
There was a problem hiding this comment.
Depending on the use case, this can be better served by a channel watch and a background goroutine, but that's definitely the more advanced version :)
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
k8s-ci-robot
added
the
lifecycle/stale
|
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-contributor-experience at kubernetes/community. |
k8s-ci-robot
added
lifecycle/rotten
|
@gerred @shawn-hurley @joelanford @alvaroaleman Can this get an approval please? |
|
Ah, I see. I'm not used to PR's being held up by comments marked as nits. My mistake. |
Co-authored-by: Joe Julian <me@joejulian.name>
Co-authored-by: Joe Julian <me@joejulian.name>
|
Thanks addressing the feedback @joejulian! /retest |
k8s-ci-robot
added
the
tide/merge-method-squash
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, dlipovetsky, joejulian The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
* Improve docs for manager.Options.SyncPeriod The comment tries to capture more of the thoughts exchanged in kubernetes-sigs#88. * Note that SyncPeriod applies to all controllers. Co-authored-by: Joe Julian <me@joejulian.name> * Make explanation of SyncPeriod more precise.. Co-authored-by: Joe Julian <me@joejulian.name> Co-authored-by: Joe Julian <me@joejulian.name>
The comment tries to capture more of the thoughts exchanged in #88.
It explains why a period sync happens. It also addresses two (common?) reasons for making the period shorter, and recommends an alternative.
The current comment includes this warning: "Change this value only if you know what you are doing." I found this unsatisfying, and wanted to add more context to the comment. I'm not an authority on the behavior/history of
SyncPeriod, and I welcome feedback 🙂.