fix(service): raise exception on uninitialized projects (#1624)

* fix(service): raise exception on uninitialized projects

conftest.py

@@ -50,6 +50,9 @@
 )
 
 IT_REMOTE_REPO_URL = os.getenv("IT_REMOTE_REPOSITORY", "https://dev.renku.ch/gitlab/renku-qa/core-integration-test")
+IT_REMOTE_NON_RENKU_REPO_URL = os.getenv(
+    "IT_REMOTE_NON_RENKU_REPO_URL", "https://dev.renku.ch/gitlab/renku-qa/core-it-non-renku"
+)
 IT_GIT_ACCESS_TOKEN = os.getenv("IT_OAUTH_GIT_TOKEN")
 
 
@@ -59,6 +62,12 @@ def it_remote_repo():
     return IT_REMOTE_REPO_URL
 
 
+@pytest.fixture(scope="module")
+def it_remote_non_renku_repo():
+    """Returns a remote path to integration test repository."""
+    return IT_REMOTE_NON_RENKU_REPO_URL
+
+
 @contextlib.contextmanager
 def _isolated_filesystem(tmpdir, name=None, delete=True):
     """Click CliRunner ``isolated_filesystem`` but xdist compatible."""
@@ -329,6 +338,7 @@ def project_metadata(project):
         "owner": "me",
         "token": "awesome token",
         "git_url": "git@gitlab.com",
+        "initialized": True,
     }
 
     yield project, metadata

renku/core/management/clone.py

@@ -95,4 +95,4 @@ def clone(
         except GitCommandError as e:
             raise errors.GitError("Cannot install Git LFS") from e
 
-    return repo
+    return repo, bool(client.project)

renku/core/management/datasets.py

@@ -1043,7 +1043,7 @@ def checkout(repo, ref):
             except PermissionError:
                 raise errors.InvalidFileOperation("Cannot delete files in {}: Permission denied".format(repo_path))
 
-        repo = clone(url, path=str(repo_path), install_githooks=False)
+        repo, _ = clone(url, path=str(repo_path), install_githooks=False)
 
         # Because the name of the default branch is not always 'master', we
         # create an alias of the default branch when cloning the repo. It

renku/service/cache/models/project.py

@@ -21,7 +21,7 @@
 import time
 from datetime import datetime
 
-from walrus import DateTimeField, IntegerField, Model, TextField
+from walrus import BooleanField, DateTimeField, IntegerField, Model, TextField
 
 from renku.service.cache.base import BaseCache
 from renku.service.config import CACHE_PROJECTS_PATH
@@ -45,6 +45,7 @@ class Project(Model):
     email = TextField()
     owner = TextField()
     token = TextField()
+    initialized = BooleanField()
 
     @property
     def abs_path(self):

renku/service/cache/serializers/project.py

@@ -40,6 +40,7 @@ class ProjectSchema(Schema):
     email = fields.String(required=True)
     owner = fields.String(required=True)
     token = fields.String(required=True)
+    initialized = fields.Boolean(default=False)
 
     @post_load
     def make_project(self, data, **options):

renku/service/controllers/api/mixins.py

@@ -22,7 +22,9 @@
 
 from git import Repo
 
-from renku.core.errors import RenkuException
+from renku.core.errors import RenkuException, UninitializedProject
+from renku.core.management.config import RENKU_HOME
+from renku.core.management.repository import RepositoryApiMixin
 from renku.core.utils.contexts import chdir
 from renku.service.controllers.utils.remote_project import RemoteProject
 
@@ -65,6 +67,10 @@ def execute_op(self):
     def local(self):
         """Execute operation against service cache."""
         project = self.cache.get_project(self.user, self.context["project_id"])
+
+        if not project.initialized:
+            raise UninitializedProject(project.abs_path)
+
         self.project_path = project.abs_path
 
         with chdir(self.project_path):
@@ -76,6 +82,10 @@ def remote(self):
 
         with project.remote() as path:
             self.project_path = Path(path)
+
+            if not (self.project_path / RENKU_HOME / RepositoryApiMixin.METADATA).exists():
+                raise UninitializedProject(self.project_path)
+
             return self.renku_op()
 
 

renku/service/controllers/utils/project_clone.py

@@ -43,7 +43,7 @@ def user_project_clone(cache, user_data, project_data):
 
     local_path.mkdir(parents=True, exist_ok=True)
 
-    repo = project_clone(
+    repo, initialized = project_clone(
         project_data["url_with_auth"],
         local_path,
         depth=project_data["depth"] if project_data["depth"] != 0 else None,
@@ -52,6 +52,8 @@ def user_project_clone(cache, user_data, project_data):
         checkout_rev=project_data["ref"],
     )
 
+    project_data["initialized"] = initialized
+
     service_log.debug(f"project successfully cloned: {repo}")
     service_log.debug(f"project folder exists: {local_path.exists()}")
 

renku/service/serializers/cache.py

@@ -155,6 +155,7 @@ class ProjectCloneResponse(Schema):
 
     project_id = fields.String(required=True)
     git_url = fields.String(required=True)
+    initialized = fields.Boolean(default=False)
 
 
 class ProjectCloneResponseRPC(JsonRPCResponse):

renku/service/views/decorators.py

@@ -33,6 +33,7 @@
     MigrationRequired,
     RenkuException,
     TemplateUpdateError,
+    UninitializedProject,
 )
 from renku.service.cache import cache
 from renku.service.config import (
@@ -146,6 +147,9 @@ def decorated_function(*args, **kwargs):
                 "reason": str(e),
             }
 
+            if isinstance(e, UninitializedProject):
+                err_response["project_initialization_required"] = True
+
             if isinstance(e, MigrationRequired):
                 err_response["migration_required"] = True
 

tests/cli/test_integration_datasets.py

@@ -1236,7 +1236,7 @@ def test_renkulab_clone(runner, monkeypatch, url):
 def test_renkulab_clone_with_config(tmpdir, url):
     """Test cloning of a Renku repo and existence of required settings."""
     with chdir(str(tmpdir)):
-        repo = project_clone(url, config={"user.name": "sam", "user.email": "s@m.i", "filter.lfs.custom": "0"})
+        repo, _ = project_clone(url, config={"user.name": "sam", "user.email": "s@m.i", "filter.lfs.custom": "0"})
 
         assert "master" == repo.active_branch.name
         reader = repo.config_reader()
@@ -1252,7 +1252,7 @@ def test_renkulab_clone_with_config(tmpdir, url):
 def test_renkulab_clone_checkout_rev(tmpdir, url):
     """Test cloning of a repo checking out a rev with static config."""
     with chdir(str(tmpdir)):
-        repo = project_clone(
+        repo, _ = project_clone(
             url,
             config={"user.name": "sam", "user.email": "s@m.i", "filter.lfs.custom": "0"},
             checkout_rev="97f907e1a3f992d4acdc97a35df73b8affc917a6",
@@ -1272,7 +1272,7 @@ def test_renkulab_clone_checkout_rev(tmpdir, url):
 def test_renku_clone_checkout_revs(tmpdir, rev):
     """Test cloning of a Renku repo checking out a rev."""
     with chdir(str(tmpdir)):
-        repo = project_clone("https://dev.renku.ch/gitlab/contact/no-renku.git", checkout_rev=rev,)
+        repo, _ = project_clone("https://dev.renku.ch/gitlab/contact/no-renku.git", checkout_rev=rev,)
 
         assert rev == repo.active_branch.name
 

tests/service/controllers/test_templates_create_project.py

@@ -49,6 +49,7 @@ def test_template_create_project_ctrl(ctrl_init, svc_client_templates_creation,
         "project_id",
         "url",
         "identifier",
+        "initialized",
         "parameters",
         "project_name",
         "name",

tests/service/jobs/test_jobs.py

@@ -139,6 +139,7 @@ def test_cleanup_project_old_keys(svc_client_cache, service_job):
         "owner": "me",
         "token": "awesome token",
         "git_url": "git@gitlab.com",
+        "initialized": True,
     }
     project = cache.make_project(user, project)
     os.makedirs(str(project.abs_path), exist_ok=True)
@@ -175,6 +176,7 @@ def test_job_constructor_lock(svc_client_cache, service_job):
         "owner": "me",
         "token": "awesome token",
         "git_url": "git@gitlab.com",
+        "initialized": True,
     }
     project = cache.make_project(user, project)
     os.makedirs(str(project.abs_path), exist_ok=True)

tests/service/test_exceptions.py

@@ -78,3 +78,34 @@ def test_migration_required_flag(svc_client_setup):
     response = svc_client.post("/datasets.create", data=json.dumps(payload), headers=headers,)
 
     assert response.json["error"]["migration_required"]
+
+
+@pytest.mark.service
+@pytest.mark.integration
+@flaky(max_runs=10, min_passes=1)
+def test_project_uninitialized(svc_client, it_remote_non_renku_repo, authentication_headers):
+    """Check migration required failure."""
+    payload = {"git_url": it_remote_non_renku_repo}
+
+    response = svc_client.post("/cache.project_clone", data=json.dumps(payload), headers=authentication_headers,)
+
+    assert response
+    assert "result" in response.json
+    assert "error" not in response.json
+
+    project_id = response.json["result"]["project_id"]
+    initialized = response.json["result"]["initialized"]
+
+    assert not initialized
+
+    payload = {
+        "project_id": project_id,
+        "name": "{0}".format(uuid.uuid4().hex),
+    }
+
+    response = svc_client.post("/datasets.create", data=json.dumps(payload), headers=authentication_headers,)
+
+    assert response
+    assert "error" in response.json
+    assert "project_initialization_required" in response.json["error"]
+    assert response.json["error"]["project_initialization_required"]

tests/service/views/test_cache_views.py

@@ -253,6 +253,7 @@ def test_clone_projects_with_auth(svc_client):
 
     assert response
     assert {"result"} == set(response.json.keys())
+    assert response.json["result"]["initialized"]
 
 
 @pytest.mark.service

tests/service/views/test_dataset_views.py

@@ -784,6 +784,7 @@ def test_import_dataset_job_enqueue(doi, svc_client_cache, project, mock_redis):
         "owner": "me",
         "token": "awesome token",
         "git_url": "git@gitlab.com",
+        "initialized": True,
     }
 
     project_obj = cache.make_project(user, project_meta)