PR#4127: handle cases where there is no event before our ts

Merges #4127
https://pagure.io/koji/pull-request/4127

Fixes: #4128
https://pagure.io/koji/issue/4128
min_event calculation can fail if koji instance is new

koji/tasks.py

@@ -889,7 +889,7 @@ class WaitrepoTask(BaseTaskHandler):
             elif isinstance(newer_than, six.integer_types + (float,)):
                 # here, we look for the first event where the tag changed after this time
                 # or, if the tag has not changed since that time, we use its last change event
-                base = self.session.getLastEvent(before=newer_than)
+                base = self.session.getLastEvent(before=newer_than, strict=False)
                 min_event = self.session.tagFirstChangeEvent(tag, after=base) or "last"
             else:
                 raise koji.GenericError("Invalid value for newer_than: %s" % newer_than)

kojihub/kojihub.py

@@ -11084,7 +11084,7 @@ class RootExports(object):
                                clauses=['id = %(id)s'], values={'id': event_id})
         return query.executeOne(strict=strict)
 
-    def getLastEvent(self, before=None):
+    def getLastEvent(self, before=None, strict=True):
         """
         Get the id and timestamp of the last event recorded in the system.
         Events are usually created as the result of a configuration change
@@ -11101,6 +11101,7 @@ class RootExports(object):
         When trying to find information about a specific event, the getEvent() method
         should be used.
         """
+        strict = convert_value(strict, cast=bool)
         fields = [
             ('id', 'id'),
             ("date_part('epoch', time)", 'ts')
@@ -11118,7 +11119,7 @@ class RootExports(object):
         opts = {'order': '-id', 'limit': 1}
         query = QueryProcessor(tables=['events'], columns=columns, aliases=aliases,
                                clauses=clauses, values=values, opts=opts)
-        return query.executeOne(strict=True)
+        return query.executeOne(strict=strict)
 
     evalPolicy = staticmethod(eval_policy)
 

kojihub/repos.py

@@ -498,7 +498,13 @@ def do_auto_requests():
         lag = lags.get(tag_id, default_lag)
         base_ts = time.time() - lag
         base_ts = (base_ts // window) * window
-        base = context.handlers.get('getLastEvent')(before=base_ts)['id']
+        base_ev = context.handlers.get('getLastEvent')(before=base_ts, strict=False)
+        if base_ev:
+            base = base_ev['id']
+        else:
+            # this will only happen with a brand new instance
+            base = kojihub.tag_first_change_event(tag_id)
+            logger.debug(f'No event older than {base_ts}, using first tag event {base}')
         check = request_repo(tag_id, min_event=min(base, last), priority=5)
         # lower priority so they don't block on-demand
         if check['duplicate']:
@@ -814,7 +820,13 @@ def default_min_event(taginfo):
     # We round base_ts to nearest window so that duplicate requests will get same event if they
     # are close in time.
     base_ts = (base_ts // window) * window
-    base = context.handlers.get('getLastEvent')(before=base_ts)['id']
+    base_ev = context.handlers.get('getLastEvent')(before=base_ts, strict=False)
+    if base_ev:
+        base = base_ev['id']
+    else:
+        # this will only happen with a brand new instance
+        base = kojihub.tag_first_change_event(taginfo['id'])
+        logger.debug(f'No event older than {base_ts}, using first tag event {base}')
     # If the tag has changed recently, we allow a bit of lag.
     # Otherwise, we use the most recent event for the tag.
     return min(base, last)

tests/test_hub/test_repo_requests.py

@@ -66,6 +66,7 @@ class BaseTest(unittest.TestCase):
         self.get_id = mock.patch('kojihub.kojihub.get_id').start()
         self.make_task = mock.patch('kojihub.kojihub.make_task').start()
         self.tag_last_change_event = mock.patch('kojihub.kojihub.tag_last_change_event').start()
+        self.tag_first_change_event = mock.patch('kojihub.kojihub.tag_first_change_event').start()
         self.query_execute = mock.MagicMock()
         self.query_executeOne = mock.MagicMock()
         self.query_singleValue = mock.MagicMock()
@@ -603,8 +604,6 @@ class TestUpdateEndEvents(BaseTest):
     def setUp(self):
         super(TestUpdateEndEvents, self).setUp()
         self.BulkUpdateProcessor = mock.patch('kojihub.repos.BulkUpdateProcessor').start()
-        self.tag_first_change_event = mock.patch('kojihub.kojihub.tag_first_change_event').start()
-        self.tag_last_change_event = mock.patch('kojihub.kojihub.tag_last_change_event').start()
 
     def test_no_update(self):
         repo = {'id': 1, 'tag_id': 99, 'create_event': 1000}
@@ -767,7 +766,7 @@ class TestAutoRequests(BaseTest):
 
         self.request_repo.called_once_with(99, min_event=1000, priority=5)
         # with zero lag, getLastEvent should be called with current time
-        self.getLastEvent.assert_called_once_with(before=now)
+        self.getLastEvent.assert_called_once_with(before=now, strict=False)
 
     def test_auto_lag_window(self):
         self.context.opts['RepoLagWindow'] = 600
@@ -792,7 +791,7 @@ class TestAutoRequests(BaseTest):
         if before > now or before < now - 600:
             raise Exception('Invalid lag calculation')
 
-    def test_no_last_event(self):
+    def test_no_last_tag_event(self):
         # corner case that should not happen
         autokeys = [
             {'tag_id': 99, 'key': 'repo.auto', 'value': 'true'},
@@ -805,6 +804,26 @@ class TestAutoRequests(BaseTest):
         self.request_repo.assert_not_called()
         self.tag_last_change_event.assert_called_once()
 
+    def test_no_last_event(self):
+        # corner case that can happen with very new instances
+        autokeys = [
+            {'tag_id': 99, 'key': 'repo.auto', 'value': 'true'},
+        ]
+        self.getLastEvent.return_value = None
+        self.query_execute.return_value = autokeys
+        self.tag_last_change_event.return_value = 1000
+        self.tag_first_change_event.return_value = 990
+        self.request_repo.return_value = {'repo': None, 'request': 'REQ', 'duplicate': False}
+
+        repos.do_auto_requests()
+
+        self.request_repo.called_once_with(99, min_event=990, priority=5)
+        self.tag_last_change_event.assert_called_once()
+        self.tag_first_change_event.assert_called_once()
+
+        repos.do_auto_requests()
+
+
 
 class TestGetRepo(BaseTest):
 
@@ -1303,6 +1322,22 @@ class TestDefaultMinEvent(BaseTest):
         base_ts = self.getLastEvent.call_args.kwargs['before']
         self.assertEqual(base_ts, now - 3600)
 
+    def test_no_last_event(self):
+        # corner case that can happen with very new instances
+        now = 1717171717
+        self.time.return_value = now
+        taginfo = {'id': 55, 'name': 'MYTAG', 'extra': {}}
+        self.tag_last_change_event.return_value = 10000
+        self.getLastEvent.return_value = None
+        self.tag_first_change_event.return_value = 9990
+
+        ev = repos.default_min_event(taginfo)
+
+        # in this corner case we should use the first event for the tag
+        self.assertEqual(ev, 9990)
+        self.getLastEvent.assert_called_once()
+        self.tag_first_change_event.assert_called_once_with(55)
+
 
 class TestCheckRequest(BaseTest):