Merge ~pappacena/turnip:async-repo-creation into turnip:master

[Thiago F. Pappacena]

Adding option on GET /repo/<repository-path> API call to check if a repository finished creation.

GET /repo/<repository-path> now returns a new key called "is_available", that should be False while the repository is being created. This could be used, for example, at Launchpad's garbo to make sure a repository creation actually failed before deleting it.

[Colin Watson (cjwatson)]

review: Needs Information

[Thiago F. Pappacena (pappacena)]

Changed the behavior of the status file as requested, and added another MP to add celery (https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/387252).

The separated MP is just to make the review a bit easier, since they should be landed at once.

[Thiago F. Pappacena (pappacena)]

Removed the code to run async repository creation from this MP. Now, this only adds the "is_available" return value to GET /repo/<repo-path>.

[Colin Watson (cjwatson)]

review: Approve

[Thiago F. Pappacena (pappacena)]

Pushed the requested change. I'll land this MP.

diff --git a/turnip/api/store.py b/turnip/api/store.py
index 5723048..f4df7e9 100644
--- a/turnip/api/store.py
+++ b/turnip/api/store.py
@@ -38,6 +38,10 @@ REF_TYPE_NAME = {
     }
 
 
+# Where to store repository status information inside a repository directory.
+REPOSITORY_CREATING_FILE_NAME = '.turnip-creating'
+
+
 def format_ref(ref, git_object):
     """Return a formatted object dict from a ref."""
     return {
@@ -209,6 +213,7 @@ def init_repo(repo_path, clone_from=None, clone_refs=False,
     if os.path.exists(repo_path):
         raise AlreadyExistsError(repo_path)
     init_repository(repo_path, is_bare)
+    set_repository_creating(repo_path, True)
 
     if clone_from:
         # The clone_from's objects and refs are in fact cloned into a
@@ -240,6 +245,7 @@ def init_repo(repo_path, clone_from=None, clone_refs=False,
         write_packed_refs(repo_path, packable_refs)
 
     ensure_config(repo_path)  # set repository configuration defaults
+    set_repository_creating(repo_path, False)
 
 
 @contextmanager
@@ -287,6 +293,29 @@ def get_default_branch(repo_path):
     return repo.references['HEAD'].target
 
 
+def set_repository_creating(repo_path, is_creating):
+    if not os.path.exists(repo_path):
+        raise ValueError("Repository %s does not exist." % repo_path)
+    file_path = os.path.join(repo_path, REPOSITORY_CREATING_FILE_NAME)
+    if is_creating:
+        open(file_path, 'a').close()
+    else:
+        try:
+            os.unlink(file_path)
+        except OSError:
+            pass
+
+
+def is_repository_available(repo_path):
+    """Checks if the repository is available (that is, if it is not in the
+    middle of a clone or init operation)."""
+    if not os.path.exists(repo_path):
+        return False
+
+    status_file_path = os.path.join(repo_path, REPOSITORY_CREATING_FILE_NAME)
+    return not os.path.exists(status_file_path)
+
+
 def set_default_branch(repo_path, target):
     repo = Repository(repo_path)
     repo.set_head(target)
diff --git a/turnip/api/tests/test_api.py b/turnip/api/tests/test_api.py
index 396369b..bcf13f9 100644
--- a/turnip/api/tests/test_api.py
+++ b/turnip/api/tests/test_api.py
@@ -98,7 +98,9 @@ class ApiTestCase(TestCase):
 
         resp = self.app.get('/repo/{}'.format(self.repo_path))
         self.assertEqual(200, resp.status_code)
-        self.assertEqual({'default_branch': 'refs/heads/branch-0'}, resp.json)
+        self.assertEqual({
+            'default_branch': 'refs/heads/branch-0',
+            'is_available': True}, resp.json)
 
     def test_repo_get_default_branch_missing(self):
         """default_branch is returned even if that branch has been deleted."""
@@ -109,7 +111,9 @@ class ApiTestCase(TestCase):
 
         resp = self.app.get('/repo/{}'.format(self.repo_path))
         self.assertEqual(200, resp.status_code)
-        self.assertEqual({'default_branch': 'refs/heads/branch-0'}, resp.json)
+        self.assertEqual({
+            'default_branch': 'refs/heads/branch-0',
+            'is_available': True}, resp.json)
 
     def test_repo_patch_default_branch(self):
         """A repository's default branch ("HEAD") can be changed."""
diff --git a/turnip/api/tests/test_store.py b/turnip/api/tests/test_store.py
index 9276303..02ed3fd 100644
--- a/turnip/api/tests/test_store.py
+++ b/turnip/api/tests/test_store.py
@@ -125,6 +125,25 @@ class InitTestCase(TestCase):
         self.assertEqual(str(yaml_config['pack.depth']),
                          repo_config['pack.depth'])
 
+    def test_is_repository_available(self):
+        repo_path = os.path.join(self.repo_store, 'repo/')
+
+        # Fail to set status if repository directory doesn't exist.
+        self.assertRaises(
+            ValueError, store.set_repository_creating, repo_path, False)
+
+        store.init_repository(repo_path, True)
+        store.set_repository_creating(repo_path, True)
+        self.assertFalse(store.is_repository_available(repo_path))
+
+        store.set_repository_creating(repo_path, False)
+        self.assertTrue(store.is_repository_available(repo_path))
+
+        # Duplicate call to set_repository_creating(False) should ignore
+        # eventually missing ".turnip-creating" file.
+        store.set_repository_creating(repo_path, False)
+        self.assertTrue(store.is_repository_available(repo_path))
+
     def test_open_ephemeral_repo(self):
         """Opening a repo where a repo name contains ':' should return
         a new ephemeral repo.
@@ -224,7 +243,10 @@ class InitTestCase(TestCase):
         # repo with the same set of refs. And the objects are copied
         # too.
         to_path = os.path.join(self.repo_store, 'to/')
+        self.assertFalse(store.is_repository_available(to_path))
         store.init_repo(to_path, clone_from=self.orig_path, clone_refs=True)
+        self.assertTrue(store.is_repository_available(to_path))
+
         to = pygit2.Repository(to_path)
         self.assertIsNot(None, to[self.master_oid])
         self.assertEqual(
@@ -260,7 +282,10 @@ class InitTestCase(TestCase):
         # init_repo with clone_from=orig and clone_refs=False creates a
         # repo without any refs, but the objects are copied.
         to_path = os.path.join(self.repo_store, 'to/')
+        self.assertFalse(store.is_repository_available(to_path))
         store.init_repo(to_path, clone_from=self.orig_path, clone_refs=False)
+        self.assertTrue(store.is_repository_available(to_path))
+
         to = pygit2.Repository(to_path)
         self.assertIsNot(None, to[self.master_oid])
         self.assertEqual([], to.listall_references())
@@ -286,7 +311,10 @@ class InitTestCase(TestCase):
 
         self.assertAllLinkCounts(1, self.orig_objs)
         to_path = os.path.join(self.repo_store, 'to/')
+        self.assertFalse(store.is_repository_available(to_path))
         store.init_repo(to_path, clone_from=self.orig_path)
+        self.assertTrue(store.is_repository_available(to_path))
+
         self.assertAllLinkCounts(2, self.orig_objs)
         to = pygit2.Repository(to_path)
         to_blob = to.create_blob(b'to')
diff --git a/turnip/api/views.py b/turnip/api/views.py
index 2e8a082..37e89ef 100644
--- a/turnip/api/views.py
+++ b/turnip/api/views.py
@@ -53,9 +53,10 @@ class RepoAPI(BaseAPI):
 
     def collection_post(self):
         """Initialise a new git repository, or clone from an existing repo."""
-        repo_path = extract_json_data(self.request).get('repo_path')
-        clone_path = extract_json_data(self.request).get('clone_from')
-        clone_refs = extract_json_data(self.request).get('clone_refs', False)
+        json_data = extract_json_data(self.request)
+        repo_path = json_data.get('repo_path')
+        clone_path = json_data.get('clone_from')
+        clone_refs = json_data.get('clone_refs', False)
 
         if not repo_path:
             self.request.errors.add('body', 'repo_path',
@@ -88,6 +89,7 @@ class RepoAPI(BaseAPI):
             raise exc.HTTPNotFound()
         return {
             'default_branch': store.get_default_branch(repo_path),
+            'is_available': store.is_repository_available(repo_path)
             }
 
     def _patch_default_branch(self, repo_path, value):