Skip to content
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

Rename Alertmanager -> GrafanaAlertmanager and remove KVStore #4

Merged
merged 2 commits into from
Jul 14, 2022

Conversation

gotjosh
Copy link
Collaborator

@gotjosh gotjosh commented Jul 14, 2022

A couple of things:

  • Renames Alertmanager to GrafanaAlertmanager
  • Introduces GrafanaAlertmanagerConfig
  • Removes the dependency on grafana's KVStore

Found a bug in the process that I've filled as #3

gotjosh added 2 commits July 14, 2022 11:55
…fanaAlertmanager

A couple of things:

- Renames `Alertmanager` to `GrafanaAlertmanager`
- Introduces `GrafanaAlertmanagerConfig`
- Removes the dependency on grafana's KVStore

Found a bug in the process that I've filled as #3
Copy link
Member

@JohnnyQQQQ JohnnyQQQQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -192,15 +194,15 @@ func newAlertmanager(ctx context.Context, orgID int64, cfg *setting.Cfg, store A

am.wg.Add(1)
go func() {
am.silences.Maintenance(silenceMaintenanceInterval, silencesFilePath, am.stopc, func() (int64, error) {
am.silences.Maintenance(config.Silences.MaintenanceFrequency(), config.Silences.Filepath(), am.stopc, func() (int64, error) {
// Delete silences older than the retention period.
if _, err := am.silences.GC(); err != nil {
am.logger.Error("silence garbage collection", "err", err)
// Don't return here - we need to snapshot our state first.
}

// Snapshot our silences to the Grafana KV store
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the comment might need an update too as it's not the Grafana KV store exclusively.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted in #8

@gotjosh gotjosh mentioned this pull request Jul 14, 2022
@gotjosh gotjosh merged commit 47df9c0 into main Jul 14, 2022
@gotjosh gotjosh deleted the remove-kv-store branch July 14, 2022 14:37
yuri-tceretian added a commit that referenced this pull request Jan 27, 2023
^ This is the 1st commit message:

refactor notifier package

move services to deciated packages
split configurations and implementations

^ The commit message #2 will be skipped:

^ move factory from Grafana

^ The commit message #3 will be skipped:

^ rename settings to config

^ The commit message #4 will be skipped:

^ reaname TemplateForTests to FroTests to appease linter

^ The commit message #5 will be skipped:

^ lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants