launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/turnip:py3-copy-ref-api into turnip:master.

Commit message:
Making copy/delete refs API compatible with py3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/393412
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/turnip:py3-copy-ref-api into turnip:master.

diff --git a/turnip/api/store.py b/turnip/api/store.py
index 28b7b45..01f4d69 100644
--- a/turnip/api/store.py
+++ b/turnip/api/store.py
@@ -28,6 +28,7 @@ from pygit2 import (
     Oid,
     Repository,
     )
+import six
 
 from turnip.config import config
 from turnip.helpers import TimeoutServerProxy
@@ -143,7 +144,10 @@ def fetch_refs(operations):
     for repo_pair, refs_pairs in grouped_refs.items():
         from_root, to_root = repo_pair
         cmd = [b'git', b'fetch', b'--no-tags', from_root]
-        cmd += [b"%s:%s" % (a, b if b else a) for a, b in refs_pairs]
+        for src, dst in refs_pairs:
+            src = six.ensure_binary(src)
+            dst = six.ensure_binary(dst if dst else src)
+            cmd.append(b"%s:%s" % (src, dst))
 
         # XXX pappacena: On Python3, this could be replaced with
         # stdout=subprocess.DEVNULL.
@@ -153,11 +157,11 @@ def fetch_refs(operations):
                 stdout=devnull, stderr=subprocess.PIPE)
             _, stderr = proc.communicate()
         if proc.returncode != 0:
-            errors.append(cmd, stderr)
+            errors.append((cmd, stderr))
 
     if errors:
-        details = b"\n ".join(b"%s = %s" % (cmd, err) for cmd, err in errors)
-        raise GitError(b"Error copying refs: %s" % details)
+        details = "\n ".join("%s = %s" % (cmd, err) for cmd, err in errors)
+        raise GitError("Error copying refs: %s" % details)
 
 
 @app.task
diff --git a/turnip/api/tests/test_api.py b/turnip/api/tests/test_api.py
index 15c366d..75a7c80 100644
--- a/turnip/api/tests/test_api.py
+++ b/turnip/api/tests/test_api.py
@@ -330,7 +330,7 @@ class ApiTestCase(TestCase, ApiRepoStoreMixin):
 
         self.assertEqual(6, len(repo.references.objects))
         self.assertEqual(200, resp.status_code)
-        self.assertEqual('', resp.body)
+        self.assertEqual(b'', resp.body)
 
     def test_delete_non_existing_ref(self):
         celery_fixture = CeleryWorkerFixture()
@@ -378,11 +378,11 @@ class ApiTestCase(TestCase, ApiRepoStoreMixin):
         body = {
             "operations": [
                 {
-                    b"from": b"refs/heads/branch-4",
-                    b"to": {b"repo": b'repo2', b"ref": b"refs/merge/123/head"}
+                    "from": "refs/heads/branch-4",
+                    "to": {"repo": 'repo2', "ref": "refs/merge/123/head"}
                 }, {
-                    b"from": b"refs/heads/branch-4",
-                    b"to": {b"repo": b'repo3', b"ref": b"refs/merge/987/head"}
+                    "from": "refs/heads/branch-4",
+                    "to": {"repo": 'repo3', "ref": "refs/merge/987/head"}
                 }]}
         resp = self.app.post_json(quote(url), body)
         self.assertEqual(202, resp.status_code)
@@ -390,13 +390,13 @@ class ApiTestCase(TestCase, ApiRepoStoreMixin):
         def branchCreated():
             repo2_refs = [i.name for i in repo2.references.objects]
             repo3_refs = [i.name for i in repo3.references.objects]
-            return (b'refs/merge/123/head' in repo2_refs and
-                    b'refs/merge/987/head' in repo3_refs)
+            return ('refs/merge/123/head' in repo2_refs and
+                    'refs/merge/987/head' in repo3_refs)
 
         celery_fixture.waitUntil(5, branchCreated)
         self.assertEqual(4, len(repo2.references.objects))
         self.assertEqual(202, resp.status_code)
-        self.assertEqual('', resp.body)
+        self.assertEqual(b'', resp.body)
 
     def test_copy_non_existing_ref(self):
         celery_fixture = CeleryWorkerFixture()
@@ -409,11 +409,11 @@ class ApiTestCase(TestCase, ApiRepoStoreMixin):
 
         body = {
             "operations": [{
-                b"from": b"refs/heads/nope",
-                b"to": {b"repo": b'repo2', b"ref": b"refs/merge/123/head"}
+                "from": "refs/heads/nope",
+                "to": {"repo": 'repo2', "ref": "refs/merge/123/head"}
             }, {
-                b"from": b"refs/heads/no-ref",
-                b"to": {b"repo": b'repo2', b"ref": b"refs/merge/123/head"}
+                "from": "refs/heads/no-ref",
+                "to": {"repo": 'repo2', "ref": "refs/merge/123/head"}
             }]}
 
         url = '/repo/repo1/refs-copy'
diff --git a/turnip/tests/tasks.py b/turnip/tests/tasks.py
index d3e065a..39dced8 100644
--- a/turnip/tests/tasks.py
+++ b/turnip/tests/tasks.py
@@ -92,7 +92,7 @@ class CeleryWorkerFixture(fixtures.Fixture):
             time.sleep(0.2)
         raise AttributeError(
             "%s(*%s, **%s) never returned True after %s seconds" %
-            (callable.func_name, args, kwargs, seconds))
+            (callable.__name__, args, kwargs, seconds))
 
     def _setUp(self):
         self.startCeleryWorker()