Merge lp:~milo/linaro-patchmetrics/bug1012037 into lp:linaro-patchmetrics

[Milo Casagrande]

This is a duplicate merge request: the previous one had to be removed since we couldn't push anything in Launchpad, and the code was in an external git repo. Now we have a Launchpad Bazaar branch here.

Pasting the old review comment:

With this merge the timeout errors should be resolved.

I re-arranged also the messages we have in case of error, in order to be a little bit more verbose so to have a better idea of where and what is happening.

I removed the file handle for stderr on the call to "git clone", since only in that case we are experiencing the timeouts. This will lead to output to be printed on the cmdline, because even redirecting it with "2>/dev/null" is not working (at least locally).

I also removed the reference to "HEAD" in git pull operations since it is only a reference, and if we have to switch branch for some reasons, this will not work.

[Deepti B. Kalakeri (deeptik)]

This looks good. Just a small comment on the alignment, can we use a similar alignment this fits well within 80 columns and looks readable as well ?

=== modified file 'apps/patchwork/utils.py'
--- apps/patchwork/utils.py 2012-06-18 14:37:25 +0000
+++ apps/patchwork/utils.py 2012-06-20 13:23:12 +0000
@@ -186,13 +186,11 @@
     root = os.path.join(settings.PATCHWORK_GIT_REPOS_DIR, project.linkname)

     if os.path.exists(root):
- proc = subprocess.Popen(
- ['git', 'pull', 'origin'], cwd=root,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+ proc = subprocess.Popen(['git', 'pull', 'origin'], cwd=root,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     else:
- proc = subprocess.Popen(
- ['git', 'clone', project.source_tree, root],
- stdout=subprocess.PIPE)
+ proc = subprocess.Popen(['git', 'clone', project.source_tree, root],
+ stdout=subprocess.PIPE)

Also, I ran pep8 on the file and there are couple of fixes which needs to be done, would you please incorporate them as well. Otherwise looks good.

review: Needs Fixing

[lp:~milo/linaro-patchmetrics/bug1012037]

365. By Milo Casagrande on 2012-06-21

Final PEP8 fixes and style.

[Milo Casagrande (milo)]

Hello Deepti,

as you suggested, I fixed the final bits about PEP8 and also the indentation style.
Thanks.

[Deepti B. Kalakeri (deeptik)]

+1. Looks good.

review: Approve

=== modified file 'apps/patchwork/utils.py'
--- apps/patchwork/utils.py	2012-01-26 15:36:57 +0000
+++ apps/patchwork/utils.py	2012-06-21 07:20:25 +0000
@@ -31,10 +31,11 @@
 from django.shortcuts import get_object_or_404
 
 
-GIT_TIMEOUT = 45 * 60
-
-
-def get_patch_ids(d, prefix = 'patch_id'):
+# Timeout for git operations.
+GIT_TIMEOUT = 55 * 60
+
+
+def get_patch_ids(d, prefix='patch_id'):
     ids = []
 
     for (k, v) in d.items():
@@ -49,6 +50,7 @@
 
     return ids
 
+
 class Order(object):
     order_map = {
         'date':         'date',
@@ -59,7 +61,7 @@
     }
     default_order = ('date', True)
 
-    def __init__(self, str = None, editable = False):
+    def __init__(self, str=None, editable=False):
         self.reversed = False
         self.editable = editable
         (self.order, self.reversed) = self.default_order
@@ -102,7 +104,10 @@
             q = '-' + q
         return q
 
+
 bundle_actions = ['create', 'add', 'remove']
+
+
 def set_bundle(user, project, action, data, patches, context):
     # set up the bundle
     bundle = None
@@ -111,24 +116,21 @@
         if not bundle_name:
             return ['No bundle name was specified']
 
-        bundle = Bundle(owner = user, project = project,
-                name = bundle_name)
+        bundle = Bundle(owner=user, project=project, name=bundle_name)
         bundle.save()
         context.add_message("Bundle %s created" % bundle.name)
-
-    elif action =='add':
-        bundle = get_object_or_404(Bundle, id = data['bundle_id'])
-
-    elif action =='remove':
-        bundle = get_object_or_404(Bundle, id = data['removed_bundle_id'])
+    elif action == 'add':
+        bundle = get_object_or_404(Bundle, id=data['bundle_id'])
+    elif action == 'remove':
+        bundle = get_object_or_404(Bundle, id=data['removed_bundle_id'])
 
     if not bundle:
         return ['no such bundle']
 
     for patch in patches:
         if action == 'create' or action == 'add':
-            bundlepatch_count = BundlePatch.objects.filter(bundle = bundle,
-                        patch = patch).count()
+            bundlepatch_count = BundlePatch.objects.filter(bundle=bundle,
+                                                           patch=patch).count()
             if bundlepatch_count == 0:
                 bundle.append_patch(patch)
                 context.add_message("Patch '%s' added to bundle %s" % \
@@ -139,7 +141,7 @@
 
         elif action == 'remove':
             try:
-                bp = BundlePatch.objects.get(bundle = bundle, patch = patch)
+                bp = BundlePatch.objects.get(bundle=bundle, patch=patch)
                 bp.delete()
                 context.add_message("Patch '%s' removed from bundle %s\n" % \
                         (patch.name, bundle.name))
@@ -184,16 +186,17 @@
 # dependencies.
 def ensure_source_checkout_for_project(project, timeout=GIT_TIMEOUT):
     root = os.path.join(settings.PATCHWORK_GIT_REPOS_DIR, project.linkname)
-    if not os.path.exists(root):
-        proc = subprocess.Popen(
-            ['git', 'clone', project.source_tree, root],
-            stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+
+    if os.path.exists(root):
+        proc = subprocess.Popen(['git', 'pull', 'origin'], cwd=root,
+                                stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     else:
-        proc = subprocess.Popen(
-            ['git', 'pull', 'origin', 'HEAD'], cwd=root,
-            stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        proc = subprocess.Popen(['git', 'clone', project.source_tree, root],
+                                stdout=subprocess.PIPE)
+
     wait = 0
     wait_increment = 5
+
     while wait < timeout:
         proc.poll()
         if proc.returncode is not None:
@@ -202,7 +205,10 @@
         time.sleep(wait_increment)
 
     if proc.returncode is None:
-        msg = "Timeout when fetching %s." % project.source_tree
+        msg = "Timeout (%d/%d seconds) when fetching '%s' (%s).\n" % \
+        (wait, timeout, project.name, project.linkname)
+        msg += " Project path: %s\n Project tree:%s\n" % \
+        (root, project.source_tree)
 
         # Kill the subprocess we started and all its children. This is
         # necessary because 'git pull' will actually run a shell script
@@ -239,6 +245,7 @@
         raise FailedToFetchGitRepo(
             "Failed to fetch %s.\nstdout:%s\nstderr:%s" % (
                 project.source_tree, stdout, stderr))
+
     return root