-
Notifications
You must be signed in to change notification settings - Fork 804
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
Remove query_store_for_labels_enabled
configuration
#5984
Remove query_store_for_labels_enabled
configuration
#5984
Conversation
@harry671003 can you help review this one? |
e32205a
to
ac89360
Compare
Signed-off-by: alanprot <alanprot@gmail.com>
ac89360
to
86e4d57
Compare
…rom ingsters Signed-off-by: alanprot <alanprot@gmail.com>
71b6a12
to
c93092b
Compare
query_store_for_labels_enabled
configurationquery_store_for_labels_enabled
configuration
@@ -103,14 +102,15 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { | |||
flagext.DeprecatedFlag(f, "querier.iterators", "Deprecated: Use iterators to execute query. This flag is no longer functional; Batch iterator is always enabled instead.", util_log.Logger) | |||
//lint:ignore faillint Need to pass the global logger like this for warning on deprecated methods | |||
flagext.DeprecatedFlag(f, "querier.batch-iterators", "Deprecated: Use batch iterators to execute query. This flag is no longer functional; Batch iterator is always enabled now.", util_log.Logger) | |||
//lint:ignore faillint Need to pass the global logger like this for warning on deprecated methods | |||
flagext.DeprecatedFlag(f, "querier.query-store-for-labels-enabled", "Deprecated: Querying long-term store is always enabled.", util_log.Logger) |
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.
Is this intended?
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.
Yeah .. thi give a warning if the flag is still present on the cli.., make sense ?
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.
Shouldn't we set this flag when we start to deprecate the flag? I think the title is a bit of misleading...
Remove query_store_for_labels_enabled configuration
we are actually deprecating the config here. Not removing.
Removing means to remove the flag entirely and it should throw error if specified
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.
Thanks Alan :)
What this PR does:
This configuration is deprecated for a long time.
This PR is removing this config from the codebase:
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]