-
Notifications
You must be signed in to change notification settings - Fork 635
Update feature gate logging to include default on #4029
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
Conversation
After the 5.7 release, when AutoUserSchemaCreate was graduated to default on/true, we discovered that our current system (and the underlying featuregate implementation) treats features explicitly turned on by the user differently than features turned on by default. This PR updates that logging to make clear that the features are those specifically requested by the user (kept as a string to help debugging) and revises the 'ShowGates' function to include those set through defaults. Issues: [PGO-1824]
cbandy
left a comment
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.
The implementation is good, but I've soured on some of the naming. I misunderstood what "String" did, and now want better method names.
I can do the work of renaming a bunch of things if you're ready to be done with this PR.
| assert.Equal(t, data.BridgeClustersTotal, 2) | ||
| assert.Equal(t, data.PGOClustersTotal, 2) | ||
| assert.Equal(t, data.FeatureGatesEnabled, "TablespaceVolumes=true") | ||
| assert.Equal(t, data.FeatureGatesEnabled, "AutoCreateUserSchema=true,TablespaceVolumes=true") |
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.
👍🏻 This isn't Contains, but I don't mind seeing the format. 🌱 We can change the test if it is ever a problem.
cbandy
left a comment
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. Please squash.
Co-authored-by: Chris Bandy <chris.bandy@crunchydata.com>
* Update feature gate logging to include default on After the 5.7 release, when AutoUserSchemaCreate was graduated to default on/true, we discovered that our current system (and the underlying featuregate implementation) treats features explicitly turned on by the user differently than features turned on by default. This PR updates that logging to make clear what features are specifically requested by the user and what features are actually enabled (a union of defaults and end user settings). Issues: [PGO-1824] Co-authored-by: Chris Bandy <chris.bandy@crunchydata.com>
Checklist:
Type of Changes:
What is the current behavior (link to any open issues here)?
After the 5.7 release, when AutoUserSchemaCreate was graduated to default on/true, we discovered that our current system (and the underlying featuregate implementation) treats features explicitly turned on by the user differently than features turned on by default.
What is the new behavior (if this is a feature change)?
This PR updates that logging to make clear that the features are those specifically requested by the user (kept as a string to help debugging) and revises the 'ShowGates' function to include those set through defaults.
Other Information:
Issues: [PGO-1824]