-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
events: Enable by default, disable flag #22815
Conversation
Build Results: |
CI Results: |
fcab3eb
to
a64cd6c
Compare
VaultExperimentCoreAuditEventsAlpha1 = "core.audit.events.alpha1" | ||
VaultExperimentSecretsSyncAlpha1 = "secrets.sync.alpha1" | ||
|
||
// Unused experiments. We keep them so that we don't break users who include them in their |
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.
I agree we should keep the experiment definition but I think we should probably generate a nice error message instead of silently ignoring - it's surprising for users if they try to enable an experiment that they think does something but it doesn't.
a64cd6c
to
998107c
Compare
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
The flag `events.alpha1` will no longer do anything, but we keep it to prevent breaking users who have it in their configurations or startup flags, or if it is referenced in other code.
998107c
to
7ab8c63
Compare
Thanks! |
The flag
events.alpha1
will no longer do anything, but we keep it to prevent breaking users who have it in their configurations or startup flags, or if it is referenced in other code.