-
Notifications
You must be signed in to change notification settings - Fork 820
Refactor rulerstore packages #3887
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
Refactor rulerstore packages #3887
Conversation
@@ -31,28 +26,3 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { | |||
cfg.ConfigDB.RegisterFlagsWithPrefix(prefix, f) | |||
cfg.RegisterFlagsWithPrefix(prefix, f) | |||
} | |||
|
|||
// NewRuleStore creates a new bucket client based on the configured backend | |||
func NewRuleStore(ctx context.Context, cfg Config, cfgProvider bucket.TenantConfigProvider, logger log.Logger, reg prometheus.Registerer) (rules.RuleStore, error) { |
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.
Moved to pkg/ruler/storage.go
to avoid circular dependencies.
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.
YES! (Assuming test and lint pass)
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.
Awesome! LGTM besides an optional suggestion
pkg/ruler/ruler.go
Outdated
"github.com/cortexproject/cortex/pkg/ruler/rulespb" | ||
store "github.com/cortexproject/cortex/pkg/ruler/rulespb" |
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.
suggestion: we may be able to trim this down
"github.com/cortexproject/cortex/pkg/ruler/rulespb" | |
store "github.com/cortexproject/cortex/pkg/ruler/rulespb" | |
"github.com/cortexproject/cortex/pkg/ruler/rulespb" |
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.
Definitely! I didn't notice it, thanks! (and done)
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, thank you.
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
d941008
to
44a3700
Compare
What this PR does:
Similarly to the package refactoring done in #3881, in this PR I'm proposing the same refactoring (same structure) for the rulestore. This should help to better reorganize packages.
The diff is huge and I'm sorry for that, but most changes have been done with the Goland IDE refactoring tool and splitted in different commits:
Which issue(s) this PR fixes:
N/A
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]