Skip to content

Conversation

@grobinson-grafana
Copy link
Collaborator

This commit changes how limits work for silences. Instead of checking if a limit is non-nil, it instead adds support for a set of no-op limits called NoopLimits.

It changes the limits in Options and Silences from Limits to *Limits because Go cannot do comparisons on structs containing function values, so we must use a nil check to see if limits are set in the options or not.

This commit changes how limits work for silences. Instead of checking
if a limit is non-nil, it instead adds support for a set of no-op
limits called NoopLimits.

It changes the limits in Options and Silences from Limits to *Limits
because Go cannot do comparisons on structs containing function
values, so we must use a nil check to see if limits are set in the
options or not.

Signed-off-by: George Robinson <george.robinson@grafana.com>
@grobinson-grafana grobinson-grafana force-pushed the grobinson/support-noop-limits branch from dcd3dc3 to 404a4af Compare June 25, 2024 14:14
@grobinson-grafana grobinson-grafana marked this pull request as draft June 25, 2024 22:52
if m := s.limits.MaxSilences(); m > 0 && len(s.st)+1 > m {
return fmt.Errorf("exceeded maximum number of silences: %d (limit: %d)", len(s.st), m)
}
if m := s.limits.MaxSilences(); m > 0 && len(s.st)+1 > m {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if we make this two functions? Then the standalone checkSizeLimits can be mostly removed as well. It also avoids having to replace the nil check with a m > 0

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not 100% sure I can see what those functions would look like, can you perhaps share an example?

@grobinson-grafana grobinson-grafana deleted the grobinson/support-noop-limits branch October 7, 2025 08:44
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