From 717a632b7d528ef19d3d04af33be27aa2315a5fa Mon Sep 17 00:00:00 2001 From: Klaas van Schelven Date: Thu, 18 Jul 2024 09:43:37 +0200 Subject: [PATCH] check_for_thresholds refactoring: 'metadata' is superfluous because it was basically the input-tuple (in a different format) --- ingest/event_counter.py | 15 ++++++--------- ingest/views.py | 9 +++++---- issues/models.py | 2 +- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/ingest/event_counter.py b/ingest/event_counter.py index 05a3ce7..b6e049a 100644 --- a/ingest/event_counter.py +++ b/ingest/event_counter.py @@ -13,7 +13,8 @@ def _filter_for_periods(qs, period_name, nr_of_periods, now): def check_for_thresholds(qs, now, thresholds, add_for_current=0): - # thresholds :: [(period_name, nr_of_periods, gte_threshold, metadata), ...] + # thresholds :: [(period_name, nr_of_periods, gte_threshold), ...] + # returns [(state, below_threshold_from, check_again_after, (period_name, nr_of_periods, gte_threshold)), ...] # This function does aggregation, so it's reasonably expensive (I haven't measured exactly, but it seems to be at # least as expensive per-call as the whole of the rest of digestion). We solve this by not calling it often, using @@ -29,9 +30,9 @@ def check_for_thresholds(qs, now, thresholds, add_for_current=0): # we only allow UTC, and we generally use Django model fields, which are UTC, so this should be good: assert now.tzinfo == timezone.utc - states_with_metadata = [] + states = [] - for (period_name, nr_of_periods, gte_threshold, metadata) in thresholds: + for (period_name, nr_of_periods, gte_threshold) in thresholds: count = _filter_for_periods(qs, period_name, nr_of_periods, now).count() + add_for_current state = count >= gte_threshold @@ -64,10 +65,6 @@ def check_for_thresholds(qs, now, thresholds, add_for_current=0): check_again_after = gte_threshold - count - states_with_metadata.append((state, below_threshold_from, check_again_after, metadata)) + states.append((state, below_threshold_from, check_again_after, (period_name, nr_of_periods, gte_threshold))) - # we return tuples of (state, below_threshold_from, check_again_after, metadata) where metadata is something - # arbitrary that can be passed in (it allows us to tie back to "what caused this to be true/false?" - # TODO: I think that in practice the metadata is always implied by the thresholds, i.e. instead of passing-through - # we could just return the thresholds that were met. - return states_with_metadata + return states diff --git a/ingest/views.py b/ingest/views.py index 6a2a37c..a020f42 100644 --- a/ingest/views.py +++ b/ingest/views.py @@ -277,8 +277,8 @@ class BaseIngestAPIView(View): # returns: True if any further processing should be done. thresholds = [ - ("minute", 5, get_settings().MAX_EVENTS_PER_PROJECT_PER_5_MINUTES, None), - ("minute", 60, get_settings().MAX_EVENTS_PER_PROJECT_PER_HOUR, None), + ("minute", 5, get_settings().MAX_EVENTS_PER_PROJECT_PER_5_MINUTES), + ("minute", 60, get_settings().MAX_EVENTS_PER_PROJECT_PER_HOUR), ] if project.quota_exceeded_until is not None and now < project.quota_exceeded_until: @@ -339,11 +339,12 @@ class BaseIngestAPIView(View): issue.next_unmute_check = issue.digested_event_count + check_again_after - for (state, until, _, vbc_dict) in states: + for (state, until, _, (period_name, nr_of_periods, gte_threshold)) in states: if not state: continue - IssueStateManager.unmute(issue, triggering_event=event, unmute_metadata={"mute_until": vbc_dict}) + IssueStateManager.unmute(issue, triggering_event=event, unmute_metadata={"mute_until": { + "period": period_name, "nr_of_periods": nr_of_periods, "volume": gte_threshold}}) # In the (in the current UI impossible, and generally unlikely) case that multiple unmute conditions are # met simultaneously, we arbitrarily break after the first. (this makes it so that a single TurningPoint diff --git a/issues/models.py b/issues/models.py index 38f3318..183d209 100644 --- a/issues/models.py +++ b/issues/models.py @@ -250,7 +250,7 @@ class IssueStateManager(object): # the for-loop in the below always contains 0 or 1 elements in our current UI (adding another unmute condition # for an already-muted issue is simply not possible) but would be robust for more elements. - return [(vbc.period, vbc.nr_of_periods, vbc.volume, vbc.to_dict()) for vbc in unmute_vbcs] + return [(vbc.period, vbc.nr_of_periods, vbc.volume) for vbc in unmute_vbcs] class IssueQuerysetStateManager(object):