-
Notifications
You must be signed in to change notification settings - Fork 5
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
docs: add a warning about caching and updates #117
Conversation
b1104ae
to
1d7a558
Compare
docs/README.md
Outdated
> the next update. | ||
> | ||
> As a general rule, we recommend you _not_ enable a cache unless the program | ||
> cannot tolerate any delay at startup. Otherwise, if you must use a cache, we |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggestion: I'd say something a bit stronger than "delay at startup" here, it's specifically being able to tolerate an outage of the secrets service, or being in the bootstrap path for same. The choice is less between "startup is fast" or "startup is slow", it's "does the thing start at all when the vault or tailscale are down"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, let me see what I can do. I'm specifically calling this out because I keep seeing caches get enabled for things where they probably shouldn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PTAL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! And yeah, this was... I think awly's worry with having a cache, that it ends up being enabled for the wrong reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, and you and I discussed that at the time. I think the tradeoff was probably worth it, because it's better to enable it for the wrong reasons than to have someone continue to use ad-hoc secrets management because of that concern.
1d7a558
to
1682dee
Compare
1682dee
to
375b612
Compare
(merging around the CI error, which is unrelated and fixed in #116) |
Updates tailscale/corp#22445