fix: adds _label and commit data to imported dataset files, single commit for imports (#651)

* fix: adds _label and commit data to imported dataset files; dataset files are imported in a single commit

* Changes bad URL reporting style

Co-Authored-By: Sam <contact@justsam.io>

Author: Panaetius

renku/api/datasets.py

@@ -145,29 +145,36 @@ def with_dataset(self, name=None):
         dataset.to_yaml()
 
     def add_data_to_dataset(
-        self, dataset, url, git=False, force=False, **kwargs
+        self, dataset, urls, git=False, force=False, **kwargs
     ):
         """Import the data into the data directory."""
         dataset_path = self.path / self.datadir / dataset.name
-        git = git or check_for_git_repo(url)
 
-        target = kwargs.pop('target', None)
+        files = []
 
-        if git:
-            if isinstance(target, (str, NoneType)):
-                files = self._add_from_git(
-                    dataset, dataset_path, url, target, **kwargs
-                )
-            else:
-                files = []
-                for t in target:
+        for url in urls:
+            git = git or check_for_git_repo(url)
+
+            target = kwargs.pop('target', None)
+
+            if git:
+                if isinstance(target, (str, NoneType)):
                     files.extend(
                         self._add_from_git(
-                            dataset, dataset_path, url, t, **kwargs
+                            dataset, dataset_path, url, target, **kwargs
                         )
                     )
-        else:
-            files = self._add_from_url(dataset, dataset_path, url, **kwargs)
+                else:
+                    for t in target:
+                        files.extend(
+                            self._add_from_git(
+                                dataset, dataset_path, url, t, **kwargs
+                            )
+                        )
+            else:
+                files.extend(
+                    self._add_from_url(dataset, dataset_path, url, **kwargs)
+                )
 
         ignored = self.find_ignored_paths(*(data['path']
                                             for data in files)) or []

renku/cli/dataset.py

@@ -289,15 +289,14 @@ def add_to_dataset(
         with client.with_dataset(name=name) as dataset:
             target = target if target else None
             with progressbar(urls, label='Adding data to dataset') as bar:
-                for url in bar:
-                    client.add_data_to_dataset(
-                        dataset,
-                        url,
-                        link=link,
-                        target=target,
-                        relative_to=relative_to,
-                        force=force,
-                    )
+                client.add_data_to_dataset(
+                    dataset,
+                    bar,
+                    link=link,
+                    target=target,
+                    relative_to=relative_to,
+                    force=force,
+                )
 
             if with_metadata:
 
@@ -310,11 +309,13 @@ def add_to_dataset(
 
                             file_.path = added_.path
                             file_.creator = with_metadata.creator
+                            file_._label = added_._label
+                            file_.commit = added_.commit
 
                 dataset.update_metadata(with_metadata)
 
     except FileNotFoundError:
-        raise BadParameter('Could not process {0}'.format(url))
+        raise BadParameter('Could not process \n{0}'.format('\n'.join(urls)))
 
 
 @dataset.command('ls-files')

renku/models/datasets.py

@@ -432,10 +432,16 @@ def update_metadata(self, other_dataset):
 
     def update_files(self, files):
         """Update files with collection of DatasetFile objects."""
-        to_insert = [
-            new_file
-            for new_file in files if not self.find_file(new_file.path)
-        ]
+        to_insert = []
+
+        for new_file in files:
+            existing_file = self.find_file(new_file.path)
+            if existing_file is None:
+                to_insert.append(new_file)
+            else:
+                existing_file.commit = new_file.commit
+                existing_file._label = new_file._label
+
         self.files += to_insert
 
     def rename_files(self, rename):

tests/test_dataset.py

@@ -77,7 +77,7 @@ def test_data_add(
                 'email': 'me@example.com',
                 'identifier': 'me_id'
             }]
-            client.add_data_to_dataset(d, '{}{}'.format(scheme, path))
+            client.add_data_to_dataset(d, ['{}{}'.format(scheme, path)])
 
         with open('data/dataset/file') as f:
             assert f.read() == '1234'
@@ -99,7 +99,7 @@ def test_data_add(
                     'identifier': 'me_id'
                 }]
                 client.add_data_to_dataset(
-                    d, '{}{}'.format(scheme, path), nocopy=True
+                    d, ['{}{}'.format(scheme, path)], nocopy=True
                 )
             assert os.path.exists('data/dataset/file')
 
@@ -113,8 +113,7 @@ def test_data_add_recursive(directory_tree, client):
             'identifier': 'me_id'
         }]
         client.add_data_to_dataset(
-            dataset,
-            directory_tree.join('dir2').strpath
+            dataset, [directory_tree.join('dir2').strpath]
         )
 
         assert os.path.basename(
@@ -134,7 +133,7 @@ def dataset_serialization(client, dataset, data_file):
 
     assert all([key in d_dict for key in ('name', 'identifier', 'files')])
     assert not len(d_dict['files'].values())
-    client.add_data_to_dataset(dataset, str(data_file))
+    client.add_data_to_dataset(dataset, [str(data_file)])
     d_dict = dataset.to_dict()
     assert len(d_dict['files'].values())
 
@@ -144,15 +143,15 @@ def test_git_repo_import(client, dataset, tmpdir, data_repository):
     # add data from local repo
     client.add_data_to_dataset(
         dataset,
-        os.path.join(os.path.dirname(data_repository.git_dir), 'dir2')
+        [os.path.join(os.path.dirname(data_repository.git_dir), 'dir2')]
     )
     assert os.stat('data/dataset/dir2/file2')
     assert dataset.files[0].path.endswith('dir2/file2')
     assert os.stat('.renku/vendors/local')
 
     # check that the creators are properly parsed from commits
     client.add_data_to_dataset(
-        dataset, os.path.dirname(data_repository.git_dir), target='file'
+        dataset, [os.path.dirname(data_repository.git_dir)], target='file'
     )
 
     assert len(dataset.files[1].creator) == 2