launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/turnip:log-by-date into turnip:master.

Commit message:
Add start_time and end_time parameters to log API

This is useful for bounding the set of commits returned in cases where
we may not have a reliable stop commit.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/turnip/+git/turnip/+merge/295544

When I was integrating git commits into Launchpad's merge proposal conversations, I initially thought about filtering by date on the turnip side, but ended up doing that on the Launchpad side instead since it was simpler.  However, it can apparently happen that our idea of the stop commit isn't in the source repository, in which case we need a different way to bound the set of commits returned by turnip's log API.

I think the OOPSes I've seen are mainly a bug in Launchpad: we need to be requesting cross-repository diffs in a better way, and I'll work on that separately.  Still, it doesn't hurt to attack this from multiple fronts.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/turnip:log-by-date into turnip:master.

diff --git a/requirements.txt b/requirements.txt
index eafd823..cc6dcf9 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -27,6 +27,7 @@ python-mimeparse==0.1.4
 # See lp:~deryck/python-openid/python-openid-fix1034376 which
 # reapplied a patch from wgrant to get codehosting going again.
 python-openid==2.2.5-fix1034376
+pytz==2014.10
 PyYAML==3.11
 repoze.lru==0.6
 simplejson==3.6.5
diff --git a/setup.py b/setup.py
index be53437..cbcfb1f 100755
--- a/setup.py
+++ b/setup.py
@@ -25,6 +25,7 @@ requires = [
     'pygit2>=0.22.1,<0.23.0',
     'python-openid',
     'PyYAML',
+    'pytz',
     'Twisted',
     'waitress',
     'zope.interface',
diff --git a/turnip/api/store.py b/turnip/api/store.py
index 1291c13..0b08fa9 100644
--- a/turnip/api/store.py
+++ b/turnip/api/store.py
@@ -6,6 +6,7 @@ from contextlib2 import (
     contextmanager,
     ExitStack,
     )
+from datetime import datetime
 import itertools
 import os
 import re
@@ -25,6 +26,7 @@ from pygit2 import (
     Oid,
     Repository,
     )
+import pytz
 
 from turnip.pack.helpers import ensure_config
 
@@ -397,12 +399,15 @@ def get_diff(repo_store, repo_name, sha1_from, sha1_to, context_lines=3):
         return diff
 
 
-def get_log(repo_store, repo_name, start=None, limit=None, stop=None):
+def get_log(repo_store, repo_name, start=None, limit=None, stop=None,
+            start_time=None, end_time=None):
     """Return a commit collection from HEAD or optionally a start oid.
 
     :param start: sha1 or branch to start listing commits from.
     :param limit: limit number of commits to return.
     :param stop: ignore a commit (and its ancestors).
+    :param start_time: ignore commits before this time (and their ancestors).
+    :param end_time: ignore commits after this time.
     """
     with open_repo(repo_store, repo_name) as repo:
         if not start:
@@ -412,7 +417,16 @@ def get_log(repo_store, repo_name, start=None, limit=None, stop=None):
             walker.hide(stop)  # filter stop sha1 and its ancestors
         if limit > 0:
             walker = itertools.islice(walker, limit)
-        return [format_commit(commit) for commit in walker]
+        for commit in walker:
+            if start_time is not None or end_time is not None:
+                commit_time = datetime.fromtimestamp(
+                    commit.committer.time, tz=pytz.UTC)
+                if start_time is not None and commit_time < start_time:
+                    walker.hide(commit)
+                    continue
+                if end_time is not None and commit_time > end_time:
+                    continue
+            yield format_commit(commit)
 
 
 def get_commit(repo_store, repo_name, revision, repo=None):
diff --git a/turnip/api/views.py b/turnip/api/views.py
index abdfaca..6e6c413 100644
--- a/turnip/api/views.py
+++ b/turnip/api/views.py
@@ -1,6 +1,7 @@
 # Copyright 2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from datetime import datetime
 import os
 import re
 from subprocess import CalledProcessError
@@ -9,6 +10,7 @@ from cornice.resource import resource
 from cornice.util import extract_json_data
 from pygit2 import GitError
 import pyramid.httpexceptions as exc
+import pytz
 
 from turnip.config import TurnipConfig
 from turnip.api import store
@@ -285,9 +287,17 @@ class LogAPI(BaseAPI):
         sha1 = self.request.matchdict['sha1']
         limit = int(self.request.params.get('limit', -1))
         stop = self.request.params.get('stop')
+        start_time = self.request.params.get('start_time')
+        if start_time is not None:
+            start_time = datetime.fromtimestamp(start_time, tz=pytz.UTC)
+        end_time = self.request.params.get('end_time')
+        if end_time is not None:
+            end_time = datetime.fromtimestamp(end_time, tz=pytz.UTC)
 
         try:
-            log = store.get_log(repo_store, repo_name, sha1, limit, stop)
+            log = list(store.get_log(
+                repo_store, repo_name, start=sha1, limit=limit, stop=stop,
+                start_time=start_time, end_time=end_time))
         except GitError:
             return exc.HTTPNotFound()
         return log