-
Notifications
You must be signed in to change notification settings - Fork 4
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
Feature Flags #508
base: main
Are you sure you want to change the base?
Feature Flags #508
Conversation
Signed-off-by: Sarah McClure <[email protected]>
Signed-off-by: Sarah McClure <[email protected]>
Unit Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 1e1bf36. |
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 have a couple of questions, but looks good to me.
client := paragliderpb.NewCloudPluginClient(conn) | ||
cloudFlags := ¶gliderpb.PluginFlags{KubernetesClustersEnabled: cfg.FeatureFlags.PluginFlags[c.Name].KubernetesClustersEnabled, | ||
PrivateEndpointsEnabled: cfg.FeatureFlags.PluginFlags[c.Name].PrivateEndpointsEnabled} | ||
_, err = client.SetFlags(context.Background(), ¶gliderpb.SetFlagsRequest{Flags: cloudFlags}) |
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.
missing logging or returning err?
AttachResourceEnabled bool `yaml:"attachResourceEnabled"` | ||
} | ||
|
||
type PluginFlags struct { |
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.
Did you consider having a flat struct containing all feature flags? I think I like the simplicity of it, plus it doesn't prevent use from having a feature flag that both orchestrator and plugins need to reason about.
TODO:
Resolves #506