fix(service): fix working on branches in the core-svc (#3472)

Panaetius · web-flow · commit 0eaf204365d3 · 2023-05-26T16:14:31.000+02:00
diff --git a/renku/ui/service/cache/models/job.py b/renku/ui/service/cache/models/job.py
@@ -43,6 +43,7 @@ class Job(Model):
     state = TextField()
     extras = JSONField()
     client_extras = TextField()
+    branch = TextField()
 
     ctrl_context = JSONField()
     ctrl_result = JSONField()
diff --git a/renku/ui/service/controllers/api/mixins.py b/renku/ui/service/controllers/api/mixins.py
@@ -169,8 +169,8 @@ def execute_op(self):
                     "git_url": self.request_data["git_url"],
                 }
 
-                if "ref" in self.request_data:
-                    clone_context["ref"] = self.request_data["ref"]
+                if "branch" in self.request_data:
+                    clone_context["branch"] = self.request_data["branch"]
 
                 # NOTE: If we want to migrate project, then we need to do full clone.
                 # This operation can take very long time, and as such is expected
@@ -185,27 +185,27 @@ def execute_op(self):
                 if not project.initialized:
                     raise UninitializedProject(project.abs_path)
             else:
-                ref = self.request_data.get("ref", None)
+                branch = self.request_data.get("branch", None)
 
-                if ref:
+                if branch:
                     with Repository(project.abs_path) as repository:
-                        if ref != repository.active_branch.name:
+                        if branch != repository.active_branch.name:
                             # NOTE: Command called for different branch than the one used in cache, change branch
                             if len(repository.remotes) != 1:
                                 raise RenkuException("Couldn't find remote for project in cache.")
                             origin = repository.remotes[0]
-                            remote_branch = f"{origin}/{ref}"
+                            remote_branch = f"{origin}/{branch}"
 
                             with project.write_lock():
-                                # NOTE: Add new ref to remote branches
-                                repository.run_git_command("remote", "set-branches", "--add", origin, ref)
+                                # NOTE: Add new branch to remote branches
+                                repository.run_git_command("remote", "set-branches", "--add", origin, branch)
                                 if self.migrate_project or self.clone_depth == PROJECT_CLONE_NO_DEPTH:
-                                    repository.fetch(origin, ref)
+                                    repository.fetch(origin, branch)
                                 else:
-                                    repository.fetch(origin, ref, depth=self.clone_depth)
+                                    repository.fetch(origin, branch, depth=self.clone_depth)
 
                                 # NOTE: Switch to new ref
-                                repository.run_git_command("checkout", "--track", "-f", "-b", ref, remote_branch)
+                                repository.run_git_command("checkout", "--track", "-f", "-b", branch, remote_branch)
 
                                 # NOTE: cleanup remote branches in case a remote was deleted (fetch fails otherwise)
                                 repository.run_git_command("remote", "prune", origin)
diff --git a/renku/ui/service/controllers/cache_migrations_check.py b/renku/ui/service/controllers/cache_migrations_check.py
@@ -66,7 +66,7 @@ def _fast_op_without_cache(self):
                 ],
                 tempdir_path,
                 remote=self.ctx["git_url"],
-                ref=self.request_data.get("ref", None),
+                branch=self.request_data.get("branch", None),
                 token=self.user_data.get("token", None),
             )
             with renku_project_context(tempdir_path):
diff --git a/renku/ui/service/controllers/templates_create_project.py b/renku/ui/service/controllers/templates_create_project.py
@@ -65,7 +65,7 @@ def default_metadata(self):
 
         metadata = {
             "__template_source__": self.ctx["git_url"],
-            "__template_ref__": self.ctx["ref"],
+            "__template_ref__": self.ctx["branch"],
             "__template_id__": self.ctx["identifier"],
             "__namespace__": self.ctx["project_namespace"],
             "__repository__": self.ctx["project_repository"],
@@ -115,7 +115,7 @@ def setup_new_project(self):
 
     def setup_template(self):
         """Reads template manifest."""
-        templates_source = fetch_templates_source(source=self.ctx["git_url"], reference=self.ctx["ref"])
+        templates_source = fetch_templates_source(source=self.ctx["git_url"], reference=self.ctx["branch"])
         identifier = self.ctx["identifier"]
         try:
             self.template = templates_source.get_template(id=identifier, reference=None)
diff --git a/renku/ui/service/controllers/templates_read_manifest.py b/renku/ui/service/controllers/templates_read_manifest.py
@@ -50,7 +50,7 @@ def template_manifest(self):
         """Reads template manifest."""
         from PIL import Image
 
-        templates_source = fetch_templates_source(source=self.ctx["git_url"], reference=self.ctx["ref"])
+        templates_source = fetch_templates_source(source=self.ctx["git_url"], reference=self.ctx["branch"])
         manifest = templates_source.manifest.get_raw_content()
 
         # NOTE: convert icons to base64
diff --git a/renku/ui/service/controllers/utils/project_clone.py b/renku/ui/service/controllers/utils/project_clone.py
@@ -73,7 +73,7 @@ def user_project_clone(cache, user_data, project_data):
                     "user.email": project_data["email"],
                     "pull.rebase": False,
                 },
-                checkout_revision=project_data["ref"],
+                checkout_revision=project_data["branch"],
             )
         ).output
         project.save()
diff --git a/renku/ui/service/controllers/utils/remote_project.py b/renku/ui/service/controllers/utils/remote_project.py
@@ -45,7 +45,7 @@ def __init__(self, user_data, request_data):
         self.ctx = ProjectCloneContext().load({**user_data, **request_data}, unknown=EXCLUDE)
 
         self.git_url = self.ctx["url_with_auth"]
-        self.branch = self.ctx["ref"]
+        self.branch = self.ctx["branch"]
 
     @property
     def remote_url(self):
diff --git a/renku/ui/service/gateways/gitlab_api_provider.py b/renku/ui/service/gateways/gitlab_api_provider.py
@@ -47,11 +47,11 @@ def download_files_from_api(
         target_folder: Union[Path, str],
         remote: str,
         token: str,
-        ref: Optional[str] = None,
+        branch: Optional[str] = None,
     ):
         """Download files through a remote Git API."""
-        if not ref:
-            ref = "HEAD"
+        if not branch:
+            branch = "HEAD"
 
         target_folder = Path(target_folder)
 
@@ -82,7 +82,7 @@ def download_files_from_api(
 
             try:
                 with open(full_path, "wb") as f:
-                    project.files.raw(file_path=str(path), ref=ref, streamed=True, action=f.write)
+                    project.files.raw(file_path=str(path), ref=branch, streamed=True, action=f.write)
 
                 result_paths.append(full_path)
             except gitlab.GitlabGetError:
diff --git a/renku/ui/service/interfaces/git_api_provider.py b/renku/ui/service/interfaces/git_api_provider.py
@@ -30,7 +30,7 @@ def download_files_from_api(
         target_folder: Union[Path, str],
         remote: str,
         token: str,
-        ref: Optional[str] = None,
+        branch: Optional[str] = None,
     ):
         """Download files through a remote Git API."""
         raise NotImplementedError()
diff --git a/renku/ui/service/serializers/cache.py b/renku/ui/service/serializers/cache.py
@@ -131,7 +131,6 @@ class RepositoryCloneRequest(RemoteRepositorySchema):
     """Request schema for repository clone."""
 
     depth = fields.Integer(metadata={"description": "Git fetch depth"}, load_default=PROJECT_CLONE_DEPTH_DEFAULT)
-    ref = fields.String(metadata={"description": "Repository reference (branch, commit or tag)"}, load_default=None)
 
 
 class ProjectCloneContext(RepositoryCloneRequest):
@@ -240,18 +239,6 @@ class ProjectMigrateRequest(AsyncSchema, LocalRepositorySchema, RemoteRepository
     skip_docker_update = fields.Boolean(dump_default=False)
     skip_migrations = fields.Boolean(dump_default=False)
 
-    @pre_load()
-    def handle_ref(self, data, **kwargs):
-        """Handle ref and branch."""
-
-        # Backward compatibility: branch and ref were both used. Let's keep branch as the exposed field
-        # even if interally it gets converted to "ref" later.
-        if data.get("ref"):
-            data["branch"] = data["ref"]
-            del data["ref"]
-
-        return data
-
 
 class ProjectMigrateResponse(RenkuSyncSchema):
     """Response schema for project migrate."""
diff --git a/renku/ui/service/serializers/common.py b/renku/ui/service/serializers/common.py
@@ -19,7 +19,7 @@
 from datetime import datetime
 
 import yagup
-from marshmallow import Schema, fields, validates
+from marshmallow import Schema, fields, pre_load, validates
 
 from renku.ui.service.errors import UserRepoUrlInvalidError
 from renku.ui.service.serializers.rpc import JsonRPCResponse
@@ -52,7 +52,18 @@ def validate_git_url(self, value):
 class RemoteRepositorySchema(RemoteRepositoryBaseSchema):
     """Schema for tracking a remote repository and branch."""
 
-    branch = fields.String(metadata={"description": "Remote git branch."})
+    branch = fields.String(load_default=None, metadata={"description": "Remote git branch (or tag or commit SHA)."})
+
+    @pre_load
+    def set_branch_from_ref(self, data, **kwargs):
+        """Set `branch` field from `ref` if present."""
+        if "ref" in data and not data.get("branch"):
+            # Backward compatibility: branch and ref were both used. Let's keep branch as the exposed field
+            # even if internally it gets converted to "ref" later.
+            data["branch"] = data["ref"]
+            del data["ref"]
+
+        return data
 
 
 class AsyncSchema(Schema):
diff --git a/renku/ui/service/serializers/datasets.py b/renku/ui/service/serializers/datasets.py
@@ -32,12 +32,6 @@
 from renku.ui.service.serializers.rpc import JsonRPCResponse
 
 
-class DatasetRefSchema(Schema):
-    """Schema for specifying a reference."""
-
-    ref = fields.String(metadata={"description": "Target reference."})
-
-
 class DatasetNameSchema(Schema):
     """Schema for dataset name."""
 
@@ -53,7 +47,7 @@ class DatasetDetailsRequest(DatasetDetails):
 
 
 class DatasetCreateRequest(
-    AsyncSchema, DatasetDetailsRequest, DatasetRefSchema, LocalRepositorySchema, RemoteRepositorySchema, MigrateSchema
+    AsyncSchema, DatasetDetailsRequest, LocalRepositorySchema, RemoteRepositorySchema, MigrateSchema
 ):
     """Request schema for a dataset create view."""
 
@@ -75,7 +69,7 @@ class DatasetCreateResponseRPC(JsonRPCResponse):
 
 
 class DatasetRemoveRequest(
-    AsyncSchema, DatasetNameSchema, DatasetRefSchema, LocalRepositorySchema, RemoteRepositorySchema, MigrateSchema
+    AsyncSchema, DatasetNameSchema, LocalRepositorySchema, RemoteRepositorySchema, MigrateSchema
 ):
     """Request schema for a dataset remove."""
 
@@ -99,9 +93,7 @@ class DatasetAddFile(Schema):
     job_id = fields.String()
 
 
-class DatasetAddRequest(
-    AsyncSchema, DatasetNameSchema, DatasetRefSchema, LocalRepositorySchema, RemoteRepositorySchema, MigrateSchema
-):
+class DatasetAddRequest(AsyncSchema, DatasetNameSchema, LocalRepositorySchema, RemoteRepositorySchema, MigrateSchema):
     """Request schema for a dataset add file view."""
 
     files = fields.List(fields.Nested(DatasetAddFile), required=True)
@@ -203,7 +195,6 @@ class DatasetEditRequest(
     AsyncSchema,
     DatasetDetailsRequest,
     DatasetNameSchema,
-    DatasetRefSchema,
     LocalRepositorySchema,
     RemoteRepositorySchema,
     MigrateSchema,
@@ -240,7 +231,7 @@ class DatasetEditResponseRPC(JsonRPCResponse):
 
 
 class DatasetUnlinkRequest(
-    AsyncSchema, DatasetNameSchema, DatasetRefSchema, LocalRepositorySchema, RemoteRepositorySchema, MigrateSchema
+    AsyncSchema, DatasetNameSchema, LocalRepositorySchema, RemoteRepositorySchema, MigrateSchema
 ):
     """Dataset unlink file request."""
 
diff --git a/renku/ui/service/serializers/templates.py b/renku/ui/service/serializers/templates.py
@@ -32,7 +32,6 @@ class ManifestTemplatesRequest(RepositoryCloneRequest):
     """Request schema for listing manifest templates."""
 
     url = fields.String(required=True)
-    ref = fields.String(load_default=None)
     depth = fields.Integer(load_default=TEMPLATE_CLONE_DEPTH_DEFAULT)
 
     @pre_load()
diff --git a/tests/fixtures/templates.py b/tests/fixtures/templates.py
@@ -59,7 +59,7 @@ def template(template_metadata):
         "url": "https://github.com/SwissDataScienceCenter/renku-project-template",
         "id": "python-minimal",
         "index": 1,
-        "ref": "master",
+        "branch": "master",
         # TODO: Add template parameters here once parameters are added to the template.
         "metadata": {},
         "default_metadata": template_metadata,
@@ -84,15 +84,15 @@ def project_init(template):
         "init_custom": [
             "init",
             "--template-ref",
-            template["ref"],
+            template["branch"],
             "--template-id",
             "python-minimal",
             data["test_project"],
         ],
         "init_custom_template": (
             "https://gitlab.dev.renku.ch/renku-python-integration-tests/core-it-template-variable-test-project"
         ),
-        "remote": ["--template-source", template["url"], "--template-ref", template["ref"]],
+        "remote": ["--template-source", template["url"], "--template-ref", template["branch"]],
         "id": ["--template-id", template["id"]],
         "force": ["--force"],
         "parameters": ["--parameter", "p1=v1", "--parameter", "p2=v2"],
diff --git a/tests/service/controllers/test_templates_create_project.py b/tests/service/controllers/test_templates_create_project.py
@@ -62,7 +62,7 @@ def test_template_create_project_ctrl(ctrl_init, svc_client_templates_creation,
         "git_url",
         "project_name_stripped",
         "depth",
-        "ref",
+        "branch",
         "new_project_url_with_auth",
         "url_with_auth",
     }
@@ -83,7 +83,7 @@ def test_template_create_project_ctrl(ctrl_init, svc_client_templates_creation,
         expected_metadata.add("__renku_version__")
     assert expected_metadata == set(received_metadata.keys())
     assert payload["url"] == received_metadata["__template_source__"]
-    assert payload["ref"] == received_metadata["__template_ref__"]
+    assert payload["branch"] == received_metadata["__template_ref__"]
     assert payload["identifier"] == received_metadata["__template_id__"]
     assert payload["project_namespace"] == received_metadata["__namespace__"]
     assert payload["project_repository"] == received_metadata["__repository__"]
diff --git a/tests/service/controllers/utils/test_remote_project.py b/tests/service/controllers/utils/test_remote_project.py
@@ -55,13 +55,13 @@ def test_project_metadata_custom_remote():
 
     request_data = {
         "git_url": "https://gitlab.dev.renku.ch/renku-python-integration-tests/import-me",
-        "ref": "my-branch",
+        "branch": "my-branch",
     }
 
     ctrl = RemoteProject(user_data, request_data)
-    ref = ctrl.ctx["ref"]
+    branch = ctrl.ctx["branch"]
 
-    assert request_data["ref"] == ref
+    assert request_data["branch"] == branch
 
 
 def test_project_metadata_remote_err():
diff --git a/tests/service/jobs/test_datasets.py b/tests/service/jobs/test_datasets.py
diff --git a/tests/service/jobs/test_project.py b/tests/service/jobs/test_project.py
diff --git a/tests/service/views/test_templates_views.py b/tests/service/views/test_templates_views.py

Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,7 @@ def _fast_op_without_cache(self):`
`66`	`66`	`],`
`67`	`67`	`tempdir_path,`
`68`	`68`	`remote=self.ctx["git_url"],`
`69`		`- ref=self.request_data.get("ref", None),`
	`69`	`+ branch=self.request_data.get("branch", None),`
`70`	`70`	`token=self.user_data.get("token", None),`
`71`	`71`	`)`
`72`	`72`	`with renku_project_context(tempdir_path):`
Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ def user_project_clone(cache, user_data, project_data):`
`73`	`73`	`"user.email": project_data["email"],`
`74`	`74`	`"pull.rebase": False,`
`75`	`75`	`},`
`76`		`- checkout_revision=project_data["ref"],`
	`76`	`+ checkout_revision=project_data["branch"],`
`77`	`77`	`)`
`78`	`78`	`).output`
`79`	`79`	`project.save()`