You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2013/12/18 23:10:44 UTC

[1/6] git commit: [#6821] Refactored common last commit logic up and removed merge commit skipping

Updated Branches:
  refs/heads/master db19ef7a4 -> 79df9b717


[#6821] Refactored common last commit logic up and removed merge commit skipping

Signed-off-by: Cory Johns <cj...@slashdotmedia.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/4070bed5
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/4070bed5
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/4070bed5

Branch: refs/heads/master
Commit: 4070bed57cd5878f2d63e14009f958d724951956
Parents: 852f084
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Mon Dec 16 18:25:14 2013 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Wed Dec 18 22:10:33 2013 +0000

----------------------------------------------------------------------
 Allura/allura/model/repository.py   | 46 +++++++++++++++++++++++++
 ForgeGit/forgegit/model/git_repo.py | 59 ++++++--------------------------
 2 files changed, 56 insertions(+), 49 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/4070bed5/Allura/allura/model/repository.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/repository.py b/Allura/allura/model/repository.py
index fa36e73..169b365 100644
--- a/Allura/allura/model/repository.py
+++ b/Allura/allura/model/repository.py
@@ -32,6 +32,8 @@ from collections import defaultdict
 from itertools import izip
 from urlparse import urljoin
 from urllib import quote
+from threading import Thread
+from Queue import Queue
 
 import tg
 from paste.deploy.converters import asbool, asint
@@ -245,6 +247,50 @@ class RepositoryImplementation(object):
     def tags(self):
         raise NotImplementedError, 'tags'
 
+    def last_commit_ids(self, commit, paths):
+        """
+        Find the ID of the last commit to touch each path.
+        """
+        if not paths:
+            return {}
+        timeout = float(tg.config.get('lcd_timeout', 60))
+        start_time = time()
+        paths = list(set(paths))  # remove dupes
+        result = {}  # will be appended to from each thread
+        chunks = Queue()
+        lcd_chunk_size = asint(tg.config.get('lcd_thread_chunk_size', 10))
+        num_threads = 0
+        for s in range(0, len(paths), lcd_chunk_size):
+            chunks.put(paths[s:s+lcd_chunk_size])
+            num_threads += 1
+        def get_ids():
+            paths = set(chunks.get())
+            try:
+                commit_id = commit._id
+                while paths and commit_id:
+                    if time() - start_time > timeout:
+                        log.error('last_commit_ids timeout for %s on %s', commit._id, ', '.join(paths))
+                        break
+                    commit_id, changes = self._get_last_commit(commit._id, paths)
+                    if commit_id is None:
+                        break
+                    changed = prefix_paths_union(paths, changes)
+                    for path in changed:
+                        result[path] = commit_id
+                    paths -= changed
+            except Exception as e:
+                log.exception('Error in SCM thread: %s', e)
+            finally:
+                chunks.task_done()
+        if num_threads == 1:
+            get_ids()
+        else:
+            for i in range(num_threads):
+                t = Thread(target=get_ids)
+                t.start()
+            chunks.join()
+        return result
+
 class Repository(Artifact, ActivityObject):
     BATCH_SIZE=100
     class __mongometa__:

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/4070bed5/ForgeGit/forgegit/model/git_repo.py
----------------------------------------------------------------------
diff --git a/ForgeGit/forgegit/model/git_repo.py b/ForgeGit/forgegit/model/git_repo.py
index f08fdd4..229f2bb 100644
--- a/ForgeGit/forgegit/model/git_repo.py
+++ b/ForgeGit/forgegit/model/git_repo.py
@@ -27,8 +27,6 @@ from datetime import datetime
 from glob import glob
 import gzip
 from time import time
-from threading import Thread
-from Queue import Queue
 
 import tg
 import git
@@ -500,54 +498,17 @@ class GitImplementation(M.RepositoryImplementation):
         self._repo.default_branch_name = name
         session(self._repo).flush(self._repo)
 
-    def last_commit_ids(self, commit, paths):
-        """
-        Find the ID of the last commit to touch each path.
-        """
-        if not paths:
-            return {}
-        timeout = float(tg.config.get('lcd_timeout', 60))
-        start_time = time()
-        paths = list(set(paths))  # remove dupes
-        result = {}  # will be appended to from each thread
-        chunks = Queue()
-        lcd_chunk_size = asint(tg.config.get('lcd_thread_chunk_size', 10))
-        num_threads = 0
-        for s in range(0, len(paths), lcd_chunk_size):
-            chunks.put(paths[s:s+lcd_chunk_size])
-            num_threads += 1
-        def get_ids():
-            paths = set(chunks.get())
-            try:
-                commit_id = commit._id
-                while paths and commit_id:
-                    if time() - start_time > timeout:
-                        log.error('last_commit_ids timeout for %s on %s', commit._id, ', '.join(paths))
-                        break
-                    lines = self._git.git.log(
-                            commit._id, '--', *paths,
-                            pretty='format:%H',
-                            name_only=True,
-                            max_count=1,
-                            no_merges=True).split('\n')
-                    commit_id = lines[0]
-                    changes = set(lines[1:])
-                    changed = prefix_paths_union(paths, changes)
-                    for path in changed:
-                        result[path] = commit_id
-                    paths -= changed
-            except Exception as e:
-                log.exception('Error in Git thread: %s', e)
-            finally:
-                chunks.task_done()
-        if num_threads == 1:
-            get_ids()
+    def _get_last_commit(self, commit_id, paths):
+        output = self._git.git.log(
+                commit_id, '--', *paths,
+                pretty='format:%H',
+                name_only=True,
+                max_count=1)
+        if not output:
+            return None, set()
         else:
-            for i in range(num_threads):
-                t = Thread(target=get_ids)
-                t.start()
-            chunks.join()
-        return result
+            lines = output.split('\n')
+            return lines[0], set(lines[1:])
 
 class _OpenedGitBlob(object):
     CHUNK_SIZE=4096


[3/6] git commit: [#6821] Remove old last_commit_ids method and fix handling of merge commits in git

Posted by br...@apache.org.
[#6821] Remove old last_commit_ids method and fix handling of merge commits in git

Signed-off-by: Cory Johns <cj...@slashdotmedia.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/f583e6e3
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/f583e6e3
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/f583e6e3

Branch: refs/heads/master
Commit: f583e6e3c2fe86fc0ffb2d0a6e25193c930cd610
Parents: e8d402b
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Tue Dec 17 00:51:26 2013 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Wed Dec 18 22:10:34 2013 +0000

----------------------------------------------------------------------
 Allura/allura/model/repository.py   | 64 +++++++++++---------------------
 ForgeGit/forgegit/model/git_repo.py | 34 ++++++++++++-----
 2 files changed, 47 insertions(+), 51 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/f583e6e3/Allura/allura/model/repository.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/repository.py b/Allura/allura/model/repository.py
index 2aeef64..10f2950 100644
--- a/Allura/allura/model/repository.py
+++ b/Allura/allura/model/repository.py
@@ -142,45 +142,6 @@ class RepositoryImplementation(object):
         '''Create a tarball for the revision'''
         raise NotImplementedError, 'tarball'
 
-    def last_commit_ids(self, commit, paths):
-        '''
-        Return a mapping {path: commit_id} of the _id of the last
-        commit to touch each path, starting from the given commit.
-        '''
-        orig_commit = commit
-        timeout = float(config.get('lcd_timeout', 60))
-        start_time = time()
-        paths = set(paths)
-        result = {}
-        seen_commits = set()
-        while paths and commit:
-            if time() - start_time > timeout:
-                log.error('last_commit_ids timeout for %s on %s', orig_commit._id, ', '.join(paths))
-                return result
-            seen_commits.add(commit._id)
-            changed = paths & set(commit.changed_paths)
-            result.update({path: commit._id for path in changed})
-            paths = paths - changed
-
-            # Hacky work-around for DiffInfoDocs previously having been
-            # computed wrong (not including children of added trees).
-            # Can be removed once all projects have had diffs / LCDs refreshed.
-            parent = commit.get_parent()
-            if parent and parent._id in seen_commits:
-                log.error('last_commit_ids loop detected at %s for %s on %s', parent._id, orig_commit._id, ', '.join(paths))
-                return result  # sanity check for bad data (loops)
-            if parent:
-                changed = set([path for path in paths if not parent.has_path(path)])
-                result.update({path: commit._id for path in changed})
-                paths = paths - changed
-            else:
-                result.update({path: commit._id for path in paths})
-                paths = set()
-            # end hacky work-around
-
-            commit = parent
-        return result
-
     def is_empty(self):
         '''Determine if the repository is empty by checking the filesystem'''
         raise NotImplementedError, 'is_empty'
@@ -248,9 +209,17 @@ class RepositoryImplementation(object):
         raise NotImplementedError, 'tags'
 
     def last_commit_ids(self, commit, paths):
-        """
-        Find the ID of the last commit to touch each path.
-        """
+        '''
+        Return a mapping {path: commit_id} of the _id of the last
+        commit to touch each path, starting from the given commit.
+
+        Chunks the set of paths based on lcd_thread_chunk_size and
+        runs each chunk (if more than one) in a separate thread.
+
+        Each thread will call :meth:`_get_last_commit` to get the
+        commit ID and list of changed files for the last commit
+        to touch any file in a given chunk.
+        '''
         if not paths:
             return {}
         timeout = float(tg.config.get('lcd_timeout', 60))
@@ -300,6 +269,17 @@ class RepositoryImplementation(object):
                 chunks.all_tasks_done.release()
         return result
 
+    def _get_last_commit(self, commit_id, paths):
+        """
+        For a given commit ID and set of paths / files,
+        use the SCM to determine the last commit to touch
+        any of the given paths / files.
+
+        Should return a tuple containing the ID of the
+        commit and the list of all files changed in the commit.
+        """
+        raise NotImplementedError('_get_last_commit')
+
 class Repository(Artifact, ActivityObject):
     BATCH_SIZE=100
     class __mongometa__:

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/f583e6e3/ForgeGit/forgegit/model/git_repo.py
----------------------------------------------------------------------
diff --git a/ForgeGit/forgegit/model/git_repo.py b/ForgeGit/forgegit/model/git_repo.py
index 229f2bb..5a2c3b9 100644
--- a/ForgeGit/forgegit/model/git_repo.py
+++ b/ForgeGit/forgegit/model/git_repo.py
@@ -499,16 +499,32 @@ class GitImplementation(M.RepositoryImplementation):
         session(self._repo).flush(self._repo)
 
     def _get_last_commit(self, commit_id, paths):
-        output = self._git.git.log(
-                commit_id, '--', *paths,
-                pretty='format:%H',
-                name_only=True,
-                max_count=1)
-        if not output:
-            return None, set()
-        else:
+        # git apparently considers merge commits to have "touched" a path
+        # if the path is changed in either branch being merged, even though
+        # the --name-only output doesn't include those files.  So, we have
+        # to filter out the merge commits that don't actually include any
+        # of the referenced paths in the list of files.
+        files = []
+        # don't skip first commit we're called with because it might be
+        # a valid change commit; however...
+        skip = 0
+        while commit_id and not files:
+            output = self._git.git.log(
+                    commit_id, '--', *paths,
+                    pretty='format:%H',
+                    name_only=True,
+                    max_count=1,
+                    skip=skip)
             lines = output.split('\n')
-            return lines[0], set(lines[1:])
+            commit_id = lines[0]
+            files = prefix_paths_union(paths, set(lines[1:]))
+            # *do* skip subsequent merge commits or we'll get stuck on an infinite
+            # loop matching and then diregarding the merge commit over and over
+            skip = 1
+        if commit_id:
+            return commit_id, files
+        else:
+            return None, set()
 
 class _OpenedGitBlob(object):
     CHUNK_SIZE=4096


[4/6] git commit: [#6821] Remove LCD dependency on DiffInfoDoc

Posted by br...@apache.org.
[#6821] Remove LCD dependency on DiffInfoDoc

Signed-off-by: Cory Johns <cj...@slashdotmedia.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/fd23eda9
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/fd23eda9
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/fd23eda9

Branch: refs/heads/master
Commit: fd23eda9f16d197e6483f80a71758bf16467b3ca
Parents: f583e6e
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Tue Dec 17 21:30:11 2013 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Wed Dec 18 22:10:34 2013 +0000

----------------------------------------------------------------------
 Allura/allura/model/repo.py         | 23 +++++++++++------------
 Allura/allura/model/repository.py   |  8 ++++++++
 ForgeGit/forgegit/model/git_repo.py |  7 +++++++
 ForgeSVN/forgesvn/model/svn.py      | 13 +++++++++++++
 4 files changed, 39 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/fd23eda9/Allura/allura/model/repo.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/repo.py b/Allura/allura/model/repo.py
index 682670a..9977a0f 100644
--- a/Allura/allura/model/repo.py
+++ b/Allura/allura/model/repo.py
@@ -430,18 +430,17 @@ class Commit(RepoObject, ActivityObject):
             If the file /foo/bar is changed in the commit,
             this would return ['', 'foo', 'foo/bar']
         '''
-        diff_info = DiffInfoDoc.m.get(_id=self._id)
-        diffs = set()
-        if diff_info:
-            for d in diff_info.differences:
-                node = d.name.strip('/')
-                diffs.add(node)
-                node_path = os.path.dirname(node)
-                while node_path:
-                    diffs.add(node_path)
-                    node_path = os.path.dirname(node_path)
-                diffs.add('')  # include '/' if there are any changes
-        return diffs
+        changes = self.repo.get_changes(self._id)
+        changed_paths = set()
+        for c in changes:
+            node = c.strip('/')
+            changed_paths.add(node)
+            node_path = os.path.dirname(node)
+            while node_path:
+                changed_paths.add(node_path)
+                node_path = os.path.dirname(node_path)
+            changed_paths.add('')  # include '/' if there are any changes
+        return changed_paths
 
     @LazyProperty
     def added_paths(self):

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/fd23eda9/Allura/allura/model/repository.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/repository.py b/Allura/allura/model/repository.py
index 10f2950..7d0fae0 100644
--- a/Allura/allura/model/repository.py
+++ b/Allura/allura/model/repository.py
@@ -280,6 +280,12 @@ class RepositoryImplementation(object):
         """
         raise NotImplementedError('_get_last_commit')
 
+    def get_changes(self, commit_id):
+        """
+        Return the list of files changed by a given commit.
+        """
+        raise NotImplemented('get_changes')
+
 class Repository(Artifact, ActivityObject):
     BATCH_SIZE=100
     class __mongometa__:
@@ -387,6 +393,8 @@ class Repository(Artifact, ActivityObject):
         return self._impl.compute_tree_new(commit, path)
     def last_commit_ids(self, commit, paths):
         return self._impl.last_commit_ids(commit, paths)
+    def get_changes(self, commit_id):
+        return self._impl.get_changes(commit_id)
     def is_empty(self):
         return self._impl.is_empty()
     def is_file(self, path, rev=None):

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/fd23eda9/ForgeGit/forgegit/model/git_repo.py
----------------------------------------------------------------------
diff --git a/ForgeGit/forgegit/model/git_repo.py b/ForgeGit/forgegit/model/git_repo.py
index 5a2c3b9..ae9f63e 100644
--- a/ForgeGit/forgegit/model/git_repo.py
+++ b/ForgeGit/forgegit/model/git_repo.py
@@ -526,6 +526,13 @@ class GitImplementation(M.RepositoryImplementation):
         else:
             return None, set()
 
+    def get_changes(self, commit_id):
+        return self._git.git.log(
+                commit_id,
+                name_only=True,
+                pretty='format:',
+                max_count=1).splitlines()[1:]
+
 class _OpenedGitBlob(object):
     CHUNK_SIZE=4096
 

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/fd23eda9/ForgeSVN/forgesvn/model/svn.py
----------------------------------------------------------------------
diff --git a/ForgeSVN/forgesvn/model/svn.py b/ForgeSVN/forgesvn/model/svn.py
index 9e6eb83..f4d0ac0 100644
--- a/ForgeSVN/forgesvn/model/svn.py
+++ b/ForgeSVN/forgesvn/model/svn.py
@@ -683,6 +683,19 @@ class SVNImplementation(M.RepositoryImplementation):
                 entries[path] = self._oid(info.last_changed_rev.number)
         return entries
 
+    def get_changes(self, oid):
+        rev = self._revision(oid)
+        try:
+            log_entry = self._svn.log(
+                self._url,
+                revision_start=rev,
+                limit=1,
+                discover_changed_paths=True)[0]
+        except pysvn.ClientError:
+            log.info('ClientError processing %r %r, treating as empty', oid, self._repo, exc_info=True)
+            log_entry = Object(date='', message='', changed_paths=[])
+        return [p.path for p in log_entry.changed_paths]
+
     def _path_to_root(self, path, rev=None):
         '''Return tag/branch/trunk root for given path inside svn repo'''
         if path:


[5/6] git commit: [#6821] Fixed failing tests due to last commit refactor

Posted by br...@apache.org.
[#6821] Fixed failing tests due to last commit refactor

Signed-off-by: Cory Johns <cj...@slashdotmedia.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/79df9b71
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/79df9b71
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/79df9b71

Branch: refs/heads/master
Commit: 79df9b717e34c7a566ab9214c32b384f0d66388a
Parents: fd23eda
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Wed Dec 18 19:10:40 2013 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Wed Dec 18 22:10:34 2013 +0000

----------------------------------------------------------------------
 Allura/allura/tests/model/test_repo.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/79df9b71/Allura/allura/tests/model/test_repo.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/model/test_repo.py b/Allura/allura/tests/model/test_repo.py
index ef39e4b..edc76b2 100644
--- a/Allura/allura/tests/model/test_repo.py
+++ b/Allura/allura/tests/model/test_repo.py
@@ -103,6 +103,10 @@ class TestLastCommit(unittest.TestCase):
         self.repo.shorthand_for_commit = lambda _id: _id[:6]
         self.repo.rev_to_commit_id = lambda rev: rev
         self.repo.log = self._log
+        self._changes = defaultdict(list)
+        self.repo.get_changes = lambda _id: self._changes[_id]
+        self._last_commits = [(None, set())]
+        self.repo._get_last_commit = lambda i, p: self._last_commits.pop()
         lcids = M.repository.RepositoryImplementation.last_commit_ids.__func__
         self.repo.last_commit_ids = lambda *a, **k: lcids(self.repo, *a, **k)
         c.lcid_cache = {}
@@ -151,12 +155,9 @@ class TestLastCommit(unittest.TestCase):
             )
         commit.tree = self._build_tree(commit, '/', tree_paths)
         commit.get_tree = lambda c: commit.tree
-        diffinfo = M.repo.DiffInfoDoc(dict(
-                _id=commit._id,
-                differences=[{'name': p} for p in diff_paths or tree_paths],
-            ))
-        diffinfo.m.save()
+        self._changes[commit._id].extend(diff_paths or tree_paths)
         self.repo._commits[commit._id] = commit
+        self._last_commits.append((commit._id, set(diff_paths or tree_paths)))
         return commit
 
     def _log(self, revs, path, id_only=True):
@@ -344,7 +345,7 @@ class TestLastCommit(unittest.TestCase):
         self.assertEqual(self.repo._commits[lcd.commit_id].message, commit3.message)
         self.assertEqual(lcd.commit_id, commit3._id)
         self.assertEqual(lcd.path, '')
-        self.assertEqual(len(lcd.entries), 2)
+        self.assertEqual(len(lcd.entries), 3)
         self.assertEqual(lcd.by_name['dir1'], commit2._id)
         self.assertEqual(lcd.by_name['file2'], commit3._id)
 


[6/6] git commit: [#6821] Remove any possibility of indefinite blocking and make test more clear

Posted by br...@apache.org.
[#6821] Remove any possibility of indefinite blocking and make test more clear

Signed-off-by: Cory Johns <cj...@slashdotmedia.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/e8d402ba
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/e8d402ba
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/e8d402ba

Branch: refs/heads/master
Commit: e8d402ba8283e40614ae495078156c0f1c0c3181
Parents: 4070bed
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Mon Dec 16 19:20:34 2013 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Wed Dec 18 22:10:34 2013 +0000

----------------------------------------------------------------------
 Allura/allura/model/repository.py                | 11 ++++++++++-
 ForgeGit/forgegit/tests/model/test_repository.py |  7 ++++---
 2 files changed, 14 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/e8d402ba/Allura/allura/model/repository.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/repository.py b/Allura/allura/model/repository.py
index 169b365..2aeef64 100644
--- a/Allura/allura/model/repository.py
+++ b/Allura/allura/model/repository.py
@@ -288,7 +288,16 @@ class RepositoryImplementation(object):
             for i in range(num_threads):
                 t = Thread(target=get_ids)
                 t.start()
-            chunks.join()
+            # reimplement chunks.join() but with a timeout
+            # see: http://bugs.python.org/issue9634
+            # (giving threads a bit of extra cleanup time in case they timeout)
+            chunks.all_tasks_done.acquire()
+            try:
+                endtime = time() + timeout + 0.5
+                while chunks.unfinished_tasks and endtime > time():
+                    chunks.all_tasks_done.wait(endtime - time())
+            finally:
+                chunks.all_tasks_done.release()
         return result
 
 class Repository(Artifact, ActivityObject):

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/e8d402ba/ForgeGit/forgegit/tests/model/test_repository.py
----------------------------------------------------------------------
diff --git a/ForgeGit/forgegit/tests/model/test_repository.py b/ForgeGit/forgegit/tests/model/test_repository.py
index 8569025..4f4f56f 100644
--- a/ForgeGit/forgegit/tests/model/test_repository.py
+++ b/ForgeGit/forgegit/tests/model/test_repository.py
@@ -456,13 +456,14 @@ class TestGitImplementation(unittest.TestCase):
         with h.push_config(tg.config, lcd_thread_chunk_size=1):
             self.test_last_commit_ids()
 
-    def test_last_commit_ids_threaded_error(self):
-        with h.push_config(tg.config, lcd_thread_chunk_size=1):
+    @mock.patch('forgegit.model.git_repo.GitImplementation._git', new_callable=mock.PropertyMock)
+    def test_last_commit_ids_threaded_error(self, _git):
+        with h.push_config(tg.config, lcd_thread_chunk_size=1, lcd_timeout=2):
             repo_dir = pkg_resources.resource_filename(
                 'forgegit', 'tests/data/testrename.git')
             repo = mock.Mock(full_fs_path=repo_dir)
+            _git.side_effect = ValueError
             impl = GM.git_repo.GitImplementation(repo)
-            impl._git = None
             lcds = impl.last_commit_ids(mock.Mock(_id='13951944969cf45a701bf90f83647b309815e6d5'), ['f2.txt', 'f3.txt'])
             self.assertEqual(lcds, {})
 


[2/6] git commit: [#6821] Handle errors in threaded git LCDs without indefinitely blocking request

Posted by br...@apache.org.
[#6821] Handle errors in threaded git LCDs without indefinitely blocking request

Signed-off-by: Cory Johns <cj...@slashdotmedia.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/852f084f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/852f084f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/852f084f

Branch: refs/heads/master
Commit: 852f084f2b0cd3daa7e931b0654f14903c7ce410
Parents: db19ef7
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Fri Dec 6 16:02:59 2013 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Wed Dec 18 22:10:33 2013 +0000

----------------------------------------------------------------------
 ForgeGit/forgegit/model/git_repo.py             | 41 +++++++++++---------
 .../forgegit/tests/model/test_repository.py     | 10 +++++
 requirements-sf.txt                             |  2 +-
 3 files changed, 33 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/852f084f/ForgeGit/forgegit/model/git_repo.py
----------------------------------------------------------------------
diff --git a/ForgeGit/forgegit/model/git_repo.py b/ForgeGit/forgegit/model/git_repo.py
index 145e528..f08fdd4 100644
--- a/ForgeGit/forgegit/model/git_repo.py
+++ b/ForgeGit/forgegit/model/git_repo.py
@@ -518,25 +518,28 @@ class GitImplementation(M.RepositoryImplementation):
             num_threads += 1
         def get_ids():
             paths = set(chunks.get())
-            commit_id = commit._id
-            while paths and commit_id:
-                if time() - start_time > timeout:
-                    log.error('last_commit_ids timeout for %s on %s', commit._id, ', '.join(paths))
-                    break
-                lines = self._git.git.log(
-                        commit._id, '--', *paths,
-                        pretty='format:%H',
-                        name_only=True,
-                        max_count=1,
-                        no_merges=True).split('\n')
-                commit_id = lines[0]
-                changes = set(lines[1:])
-                changed = prefix_paths_union(paths, changes)
-                for path in changed:
-                    result[path] = commit_id
-                paths -= changed
-            chunks.task_done()
-            return
+            try:
+                commit_id = commit._id
+                while paths and commit_id:
+                    if time() - start_time > timeout:
+                        log.error('last_commit_ids timeout for %s on %s', commit._id, ', '.join(paths))
+                        break
+                    lines = self._git.git.log(
+                            commit._id, '--', *paths,
+                            pretty='format:%H',
+                            name_only=True,
+                            max_count=1,
+                            no_merges=True).split('\n')
+                    commit_id = lines[0]
+                    changes = set(lines[1:])
+                    changed = prefix_paths_union(paths, changes)
+                    for path in changed:
+                        result[path] = commit_id
+                    paths -= changed
+            except Exception as e:
+                log.exception('Error in Git thread: %s', e)
+            finally:
+                chunks.task_done()
         if num_threads == 1:
             get_ids()
         else:

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/852f084f/ForgeGit/forgegit/tests/model/test_repository.py
----------------------------------------------------------------------
diff --git a/ForgeGit/forgegit/tests/model/test_repository.py b/ForgeGit/forgegit/tests/model/test_repository.py
index d2a9413..8569025 100644
--- a/ForgeGit/forgegit/tests/model/test_repository.py
+++ b/ForgeGit/forgegit/tests/model/test_repository.py
@@ -456,6 +456,16 @@ class TestGitImplementation(unittest.TestCase):
         with h.push_config(tg.config, lcd_thread_chunk_size=1):
             self.test_last_commit_ids()
 
+    def test_last_commit_ids_threaded_error(self):
+        with h.push_config(tg.config, lcd_thread_chunk_size=1):
+            repo_dir = pkg_resources.resource_filename(
+                'forgegit', 'tests/data/testrename.git')
+            repo = mock.Mock(full_fs_path=repo_dir)
+            impl = GM.git_repo.GitImplementation(repo)
+            impl._git = None
+            lcds = impl.last_commit_ids(mock.Mock(_id='13951944969cf45a701bf90f83647b309815e6d5'), ['f2.txt', 'f3.txt'])
+            self.assertEqual(lcds, {})
+
 
 class TestGitCommit(unittest.TestCase):
 

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/852f084f/requirements-sf.txt
----------------------------------------------------------------------
diff --git a/requirements-sf.txt b/requirements-sf.txt
index 81b2424..8fa5d69 100644
--- a/requirements-sf.txt
+++ b/requirements-sf.txt
@@ -4,7 +4,7 @@ akismet==0.2.0
 amqplib==0.6.1
 kombu==1.0.4
 coverage==3.5a1-20110413
-ForgeHg==0.1.18
+ForgeHg==0.1.19
 ForgePastebin==0.3.0
 GoogleCodeWikiImporter==0.4.7
 mechanize==0.2.4