launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/turnip:log-missing-stop into turnip:master with ~cjwatson/turnip:log-by-date as a prerequisite.

Commit message:
Make log API ignore a missing stop commit

Determining whether the stop commit exists in advance requires an extra
round-trip, and clients are likely to just fall back to omitting the
stop commit anyway.  start_time and end_time provide an alternative way
to bound the set of commits which can be used in addition to a
possibly-missing stop commit.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Make log API ignore a missing stop commit.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/turnip:log-missing-stop into turnip:master.

diff --git a/turnip/api/store.py b/turnip/api/store.py
index 0b08fa9..9759fd8 100644
--- a/turnip/api/store.py
+++ b/turnip/api/store.py
@@ -414,7 +414,10 @@ def get_log(repo_store, repo_name, start=None, limit=None, stop=None,
             start = repo.head.target  # walk from HEAD
         walker = repo.walk(start)
         if stop:
-            walker.hide(stop)  # filter stop sha1 and its ancestors
+            try:
+                walker.hide(stop)  # filter stop sha1 and its ancestors
+            except KeyError:
+                pass
         if limit > 0:
             walker = itertools.islice(walker, limit)
         for commit in walker:
diff --git a/turnip/api/tests/test_api.py b/turnip/api/tests/test_api.py
index 065b067..7b2511b 100644
--- a/turnip/api/tests/test_api.py
+++ b/turnip/api/tests/test_api.py
@@ -558,6 +558,15 @@ class ApiTestCase(TestCase):
         self.assertEqual(5, len(resp.json))
         self.assertNotIn(excluded_commit, resp.json)
 
+    def test_repo_get_log_with_missing_stop(self):
+        """A missing stop commit is ignored rather than crashing."""
+        factory = RepoFactory(self.repo_store, num_commits=10)
+        repo = factory.build()
+        head = repo.head.target
+        resp = self.app.get('/repo/{}/log/{}?stop={}'.format(
+            self.repo_path, head, factory.nonexistent_oid()))
+        self.assertEqual(10, len(resp.json))
+
     def test_repo_repack_verify_pack(self):
         """Ensure commit exists in pack."""
         factory = RepoFactory(self.repo_store)