Add or update from a specific commit; LFS fixes (#805)

* feat: add or update dataset files to a specific git commit

* track dataset files in LFS

* Skip LFS smudge filter

* docs: dataset add --ref

Author: mohammad-alisafaee

renku/cli/dataset.py

@@ -111,6 +111,10 @@
         new-dir/
           new-filename
 
+To add a specific version of files, use ``--ref`` option for selecting a
+branch, commit, or tag. The value passed to this option must be a valid
+reference in the remote Git repository.
+
 Tagging a dataset:
 
 A dataset can be tagged with an arbitrary tag to refer to the dataset at that
@@ -404,7 +408,10 @@ def edit(dataset_id):
     default='',
     help='Destination file or directory within the dataset path'
 )
-def add(name, urls, link, force, create, sources, destination):
+@click.option(
+    '--ref', default=None, help='Add files from a specific commit/tag/branch.'
+)
+def add(name, urls, link, force, create, sources, destination, ref):
     """Add data to a dataset."""
     progress = partial(progressbar, label='Adding data to dataset')
     add_file(
@@ -415,6 +422,7 @@ def add(name, urls, link, force, create, sources, destination):
         create=create,
         sources=sources,
         destination=destination,
+        ref=ref,
         urlscontext=progress
     )
 
@@ -638,8 +646,11 @@ def _init(lock, id_queue):
     multiple=True,
     help='Exclude files matching given pattern.'
 )
-def update(names, creators, include, exclude):
+@click.option(
+    '--ref', default=None, help='Update to a specific commit/tag/branch.'
+)
+def update(names, creators, include, exclude, ref):
     """Updates files in dataset from a remote Git repo."""
     progress_context = partial(progressbar, label='Updating files')
-    update_datasets(names, creators, include, exclude, progress_context)
+    update_datasets(names, creators, include, exclude, ref, progress_context)
     click.secho('OK', fg='green')

renku/core/commands/dataset.py

@@ -141,12 +141,13 @@ def add_file(
     create=False,
     sources=(),
     destination='',
+    ref=None,
     with_metadata=None,
     urlscontext=contextlib.nullcontext
 ):
     """Add data file to a dataset."""
     add_to_dataset(
-        client, urls, name, link, force, create, sources, destination,
+        client, urls, name, link, force, create, sources, destination, ref,
         with_metadata, urlscontext
     )
 
@@ -160,6 +161,7 @@ def add_to_dataset(
     create=False,
     sources=(),
     destination='',
+    ref=None,
     with_metadata=None,
     urlscontext=contextlib.nullcontext
 ):
@@ -180,6 +182,7 @@ def add_to_dataset(
                     force=force,
                     sources=sources,
                     destination=destination,
+                    ref=ref
                 )
 
             if with_metadata:
@@ -511,6 +514,7 @@ def update_datasets(
     creators,
     include,
     exclude,
+    ref,
     progress_context=contextlib.nullcontext
 ):
     """Update files from a remote Git repo."""
@@ -527,6 +531,7 @@ def update_datasets(
 
     datasets = {}
     possible_updates = []
+    unique_remotes = set()
 
     for file_ in records:
         if file_.based_on:
@@ -539,11 +544,19 @@ def update_datasets(
 
             file_.dataset = dataset
             possible_updates.append(file_)
+            unique_remotes.add(file_.based_on['url'])
+
+    if ref and len(unique_remotes) > 1:
+        raise ParameterError(
+            'Cannot use "--ref" with more than one Git repository.\n'
+            'Limit list of files to be updated to one repository. See'
+            '"renku dataset update -h" for more information.'
+        )
 
     with progress_context(
         possible_updates, item_show_func=lambda x: x.path if x else None
     ) as progressbar:
-        client.update_dataset_files(progressbar)
+        client.update_dataset_files(progressbar, ref)
 
 
 def _include_exclude(file_path, include=None, exclude=None):

renku/core/management/datasets.py

@@ -179,6 +179,7 @@ def add_data_to_dataset(
         force=False,
         sources=(),
         destination='',
+        ref=None,
         link=False
     ):
         """Import the data into the data directory."""
@@ -193,7 +194,7 @@ def add_data_to_dataset(
                 sources = sources or ()
                 files.extend(
                     self._add_from_git(
-                        dataset, dataset_path, url, sources, destination
+                        dataset, dataset_path, url, sources, destination, ref
                     )
                 )
             else:
@@ -313,7 +314,9 @@ def _add_from_url(self, dataset, dataset_path, url, link, destination):
             'parent': self
         }]
 
-    def _add_from_git(self, dataset, dataset_path, url, sources, destination):
+    def _add_from_git(
+        self, dataset, dataset_path, url, sources, destination, ref
+    ):
         """Process adding resources from another git repository."""
         from renku import LocalClient
 
@@ -357,7 +360,7 @@ def _add_from_git(self, dataset, dataset_path, url, sources, destination):
                 'Scheme {} not supported'.format(u.scheme)
             )
 
-        repo, repo_path = self._prepare_git_repo(url)
+        repo, repo_path = self._prepare_git_repo(url, ref)
 
         dataset_path = self.path / dataset_path
 
@@ -419,15 +422,19 @@ def _add_from_git(self, dataset, dataset_path, url, sources, destination):
                     client, path=path, url=url
                 )
 
+                path_in_dst_repo = dst.relative_to(self.path)
+
                 results.append({
-                    'path': dst.relative_to(self.path),
+                    'path': path_in_dst_repo,
                     'url': dst_url,
                     'creator': creators,
                     'dataset': dataset.name,
                     'parent': self,
                     'based_on': based_on
                 })
 
+                self.track_paths_in_storage(str(path_in_dst_repo))
+
                 dst.parent.mkdir(parents=True, exist_ok=True)
                 shutil.copy(str(src), str(dst))
 
@@ -611,7 +618,7 @@ def remove_dataset_tags(self, dataset, tags):
 
         return dataset
 
-    def update_dataset_files(self, files):
+    def update_dataset_files(self, files, ref=None):
         """Update files and dataset metadata according to their remotes."""
         from renku import LocalClient
 
@@ -625,7 +632,7 @@ def update_dataset_files(self, files):
             if url in visited_repos:
                 repo, repo_path, remote_client = visited_repos[url]
             else:
-                repo, repo_path = self._prepare_git_repo(url)
+                repo, repo_path = self._prepare_git_repo(url, ref)
                 remote_client = LocalClient(repo_path)
                 visited_repos[url] = repo, repo_path, remote_client
 
@@ -703,7 +710,19 @@ def update_dataset_files(self, files):
         for dataset in modified_datasets.values():
             dataset.to_yaml()
 
-    def _prepare_git_repo(self, url):
+    def _prepare_git_repo(self, url, ref):
+        def checkout(repo, ref):
+            try:
+                repo.git.checkout(ref)
+            except GitCommandError:
+                raise errors.ParameterError(
+                    'Cannot find reference "{}" in Git repository: {}'.format(
+                        ref, url
+                    )
+                )
+
+        RENKU_BRANCH = 'renku-default-branch'
+        ref = ref or RENKU_BRANCH
         u = GitURL.parse(url)
         path = u.pathname
         if u.hostname == 'localhost':
@@ -715,11 +734,11 @@ def _prepare_git_repo(self, url):
 
         if repo_path.exists():
             repo = Repo(str(repo_path))
-            origin = repo.remotes.origin
-            if origin.url == url:
+            if repo.remotes.origin.url == url:
                 try:
-                    origin.fetch()
-                    origin.pull()
+                    repo.git.fetch()
+                    repo.git.checkout(ref)
+                    repo.git.pull()
                 except GitError:
                     # ignore the error and try re-cloning
                     pass
@@ -735,7 +754,24 @@ def _prepare_git_repo(self, url):
                 )
 
         try:
+            os.environ['GIT_LFS_SKIP_SMUDGE'] = '1'
             repo = Repo.clone_from(url, str(repo_path), recursive=True)
+            # Because the name of the default branch is not always 'master', we
+            # create an alias of the default branch when cloning the repo. It
+            # is used to refer to the default branch later.
+            renku_ref = 'refs/heads/' + RENKU_BRANCH
+            repo.git.execute([
+                'git', 'symbolic-ref', renku_ref, repo.head.reference.path
+            ])
+            checkout(repo, ref)
+            # Disable Git LFS smudge filter
+            repo.git.execute(
+                command=[
+                    'git', 'lfs', 'install', '--local', '--skip-smudge',
+                    '--force'
+                ],
+                with_exceptions=False
+            )
         except GitCommandError:
             raise errors.GitError(
                 'Cannot access remote Git repo: {}'.format(url)

tests/cli/test_integration_datasets.py

@@ -17,6 +17,7 @@
 # limitations under the License.
 """Integration tests for dataset command."""
 import os
+import subprocess
 
 import git
 import pytest
@@ -624,3 +625,108 @@ def test_datasets_import_target(
         ],
     )
     assert 0 == result.exit_code
+
+
+@pytest.mark.integration
+@pytest.mark.parametrize(
+    'ref', ['v0.3.0', 'fe6ec65cc84bcf01e879ef38c0793208f7fab4bb']
+)
+def test_add_specific_refs(ref, runner, client):
+    """Test adding a specific version of files."""
+    FILENAME = 'CHANGES.rst'
+    # create a dataset
+    result = runner.invoke(cli, ['dataset', 'create', 'dataset'])
+    assert 0 == result.exit_code
+
+    # add data from a git repo
+    result = runner.invoke(
+        cli, [
+            'dataset', 'add', 'dataset', '-s', FILENAME, '--ref', ref,
+            'https://github.com/SwissDataScienceCenter/renku-python.git'
+        ]
+    )
+    assert 0 == result.exit_code
+    content = (client.path / 'data' / 'dataset' / FILENAME).read_text()
+    assert 'v0.3.0' in content
+    assert 'v0.3.1' not in content
+
+
+@pytest.mark.integration
+@pytest.mark.parametrize(
+    'ref', ['v0.3.1', '27e29abd409c83129a3fdb8b8b0b898b23bcb229']
+)
+def test_update_specific_refs(ref, runner, client):
+    """Test updating to a specific version of files."""
+    FILENAME = 'CHANGES.rst'
+    # create a dataset
+    result = runner.invoke(cli, ['dataset', 'create', 'dataset'])
+    assert 0 == result.exit_code
+
+    # add data from a git repo
+    result = runner.invoke(
+        cli, [
+            'dataset', 'add', 'dataset', '-s', FILENAME, '--ref', 'v0.3.0',
+            'https://github.com/SwissDataScienceCenter/renku-python.git'
+        ]
+    )
+    assert 0 == result.exit_code
+    content = (client.path / 'data' / 'dataset' / FILENAME).read_text()
+    assert 'v0.3.1' not in content
+
+    # update data to a later version
+    result = runner.invoke(cli, ['dataset', 'update', '--ref', ref])
+    assert 0 == result.exit_code
+    content = (client.path / 'data' / 'dataset' / FILENAME).read_text()
+    assert 'v0.3.1' in content
+    assert 'v0.3.2' not in content
+
+
+@pytest.mark.integration
+def test_update_with_multiple_remotes_and_ref(runner, client):
+    """Test updating fails when ref is ambiguous."""
+    # create a dataset
+    result = runner.invoke(cli, ['dataset', 'create', 'dataset'])
+    assert 0 == result.exit_code
+
+    # add data from a git repo
+    result = runner.invoke(
+        cli, [
+            'dataset', 'add', 'dataset', '-s', 'CHANGES.rst',
+            'https://github.com/SwissDataScienceCenter/renku-python.git'
+        ]
+    )
+    assert 0 == result.exit_code
+
+    # add data from another git repo
+    result = runner.invoke(
+        cli, [
+            'dataset', 'add', 'dataset', '-s', 'LICENSE',
+            'https://github.com/SwissDataScienceCenter/renku-notebooks.git'
+        ]
+    )
+    assert 0 == result.exit_code
+
+    # update data to a later version
+    result = runner.invoke(cli, ['dataset', 'update', '--ref', 'any-value'])
+    assert 2 == result.exit_code
+    assert 'Cannot use "--ref" with more than one Git repo' in result.output
+
+
+@pytest.mark.integration
+def test_files_are_tracked_in_lfs(runner, client):
+    """Test files added from a Git repo are tacked in Git LFS."""
+    FILENAME = 'CHANGES.rst'
+    # create a dataset
+    result = runner.invoke(cli, ['dataset', 'create', 'dataset'])
+    assert 0 == result.exit_code
+
+    # add data from a git repo
+    result = runner.invoke(
+        cli, [
+            'dataset', 'add', 'dataset', '-s', FILENAME,
+            'https://github.com/SwissDataScienceCenter/renku-python.git'
+        ]
+    )
+    assert 0 == result.exit_code
+    path = 'data/dataset/{}'.format(FILENAME)
+    assert path in subprocess.check_output(['git', 'lfs', 'ls-files']).decode()