fix: resync repo after import action (#1052)

conftest.py

@@ -694,7 +694,7 @@ def svc_client_with_repo(integration_lifecycle):
         current = repo.create_head(new_branch)
         current.checkout()
 
-    yield svc_client, deepcopy(headers), project_id
+    yield svc_client, deepcopy(headers), project_id, url_components
 
 
 @pytest.fixture(

renku/service/jobs/datasets.py

@@ -25,10 +25,17 @@
 from renku.service.jobs.constants import USER_JOB_STATE_COMPLETED, \
     USER_JOB_STATE_FAILED, USER_JOB_STATE_IN_PROGRESS
 from renku.service.serializers.jobs import UserJob
-from renku.service.utils import make_project_path
+from renku.service.utils import make_project_path, repo_sync
 from renku.service.views.decorators import requires_cache
 
 
+def fail_job(cache, user_job, user, error):
+    """Mark job as failed."""
+    user_job['state'] = USER_JOB_STATE_FAILED
+    user_job['extras']['error'] = error
+    cache.set_job(user, user_job)
+
+
 class DatasetImportJobProcess(DownloadProgressCallback):
     """Track dataset import job progress."""
 
@@ -80,8 +87,9 @@ def dataset_import(
     """Job for dataset import."""
     user_job = cache.get_job(user, user_job_id)
     project = cache.get_project(user, project_id)
+    project_path = make_project_path(user, project)
 
-    with chdir(make_project_path(user, project)):
+    with chdir(project_path):
         try:
             import_dataset(
                 dataset_uri,
@@ -90,9 +98,13 @@ def dataset_import(
                 progress=DatasetImportJobProcess(cache, user, user_job)
             )
         except (HTTPError, ParameterError) as exp:
-            user_job['state'] = USER_JOB_STATE_FAILED
-            user_job['extras']['error'] = str(exp)
-            cache.set_job(user, user_job)
+            fail_job(cache, user_job, user, str(exp))
 
             # Reraise exception, so we see trace in job metadata.
             raise exp
+
+    if not repo_sync(project_path):
+        error = 'failed to push refs'
+        fail_job(cache, user_job, user, error)
+
+        raise RuntimeError(error)

tests/service/jobs/test_datasets.py

@@ -16,17 +16,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 """Renku service dataset jobs tests."""
-import os
-import shutil
-import uuid
-from pathlib import Path
+import json
 
 import pytest
+from flaky import flaky
+from git import Repo
+from tests.service.views.test_dataset_views import assert_rpc_response
 
-from renku.core.commands.dataset import list_datasets
-from renku.core.utils.contexts import chdir
-from renku.service.jobs.constants import USER_JOB_STATE_COMPLETED, \
-    USER_JOB_STATE_ENQUEUED
 from renku.service.jobs.datasets import dataset_import
 from renku.service.utils import make_project_path
 
@@ -39,55 +35,40 @@
     ]
 )
 @pytest.mark.integration
-def test_dataset_import_job(doi, svc_client_cache, project):
-    """Test dataset import"""
-    client, cache = svc_client_cache
-
-    user = {'user_id': 'user'}
-
-    project_meta = {
-        'project_id': uuid.uuid4().hex,
-        'name': Path(project).name,
-        'fullname': 'full project name',
-        'email': 'my@email.com',
-        'owner': 'me',
-        'token': 'awesome token',
-        'git_url': 'git@gitlab.com'
+@flaky(max_runs=30, min_passes=1)
+def test_dataset_import_job(doi, svc_client_with_repo):
+    """Test dataset import."""
+    svc_client, headers, project_id, url_components = svc_client_with_repo
+    user = {'user_id': headers['Renku-User-Id']}
+    payload = {
+        'project_id': project_id,
+        'dataset_uri': doi,
     }
+    response = svc_client.post(
+        '/datasets.import',
+        data=json.dumps(payload),
+        headers=headers,
+    )
 
-    job_request = {
-        'job_id': uuid.uuid4().hex,
-        'state': USER_JOB_STATE_ENQUEUED,
-    }
+    assert response
+    assert_rpc_response(response)
+    assert {'job_id', 'created_at'} == set(response.json['result'].keys())
 
-    cache.create_job(user, job_request)
-    cache.set_project(user, project_meta['project_id'], project_meta)
+    dest = make_project_path(
+        user, {
+            'owner': url_components.owner,
+            'name': url_components.name
+        }
+    )
 
-    dest = make_project_path(user, project_meta)
-    os.makedirs(dest.parent, exist_ok=True)
-    if not (project / dest).exists():
-        shutil.copytree(project, dest)
+    old_commit = Repo(dest).head.commit
 
     dataset_import(
         user,
-        job_request['job_id'],
-        project_meta['project_id'],
+        response.json['result']['job_id'],
+        project_id,
         doi,
     )
 
-    with chdir(dest):
-        datasets = list_datasets()
-
-        assert datasets and isinstance(datasets, list)
-        assert doi in ';'.join([ds.same_as.url for ds in datasets])
-
-    updated_job = cache.get_job(user, job_request['job_id'])
-
-    assert USER_JOB_STATE_COMPLETED == updated_job['state']
-    assert {
-        'extras',
-        'job_id',
-        'state',
-        'created_at',
-        'updated_at',
-    } == set(updated_job.keys())
+    new_commit = Repo(dest).head.commit
+    assert old_commit.hexsha != new_commit.hexsha

tests/service/jobs/test_jobs.py

@@ -37,7 +37,7 @@ def test_cleanup_old_files(
     datapack_zip, svc_client_with_repo, service_job, mock_redis
 ):
     """Upload archive and add its contents to a dataset."""
-    svc_client, headers, _ = svc_client_with_repo
+    svc_client, headers, _, _ = svc_client_with_repo
     headers.pop('Content-Type')
 
     response = svc_client.post(
@@ -119,7 +119,7 @@ def test_cleanup_old_project(
     datapack_zip, svc_client_with_repo, service_job, mock_redis
 ):
     """Upload archive and add its contents to a dataset."""
-    svc_client, headers, _ = svc_client_with_repo
+    svc_client, headers, _, _ = svc_client_with_repo
     headers.pop('Content-Type')
 
     response = svc_client.get('/cache.project_list', headers=headers)

tests/service/views/test_cache_views.py

@@ -451,7 +451,7 @@ def test_clone_projects_invalid_headers(svc_client):
 @pytest.mark.service
 def test_upload_zip_unpack_archive(datapack_zip, svc_client_with_repo):
     """Upload zip archive with unpack."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     headers.pop('Content-Type')
 
     response = svc_client.post(
@@ -480,7 +480,7 @@ def test_upload_zip_unpack_archive(datapack_zip, svc_client_with_repo):
 @pytest.mark.service
 def test_upload_zip_archive(datapack_zip, svc_client_with_repo):
     """Upload zip archive."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     headers.pop('Content-Type')
 
     response = svc_client.post(
@@ -509,7 +509,7 @@ def test_upload_zip_archive(datapack_zip, svc_client_with_repo):
 @pytest.mark.service
 def test_upload_tar_unpack_archive(datapack_tar, svc_client_with_repo):
     """Upload zip archive with unpack."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     headers.pop('Content-Type')
 
     response = svc_client.post(
@@ -557,7 +557,7 @@ def test_upload_tar_unpack_archive(datapack_tar, svc_client_with_repo):
 @pytest.mark.service
 def test_upload_tar_archive(datapack_tar, svc_client_with_repo):
     """Upload zip archive."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     headers.pop('Content-Type')
 
     response = svc_client.post(
@@ -586,7 +586,7 @@ def test_upload_tar_archive(datapack_tar, svc_client_with_repo):
 @pytest.mark.service
 def test_field_upload_resp_fields(datapack_tar, svc_client_with_repo):
     """Check response fields."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     headers.pop('Content-Type')
 
     response = svc_client.post(

tests/service/views/test_dataset_views.py

@@ -49,7 +49,7 @@ def assert_rpc_response(response, with_key='result'):
 @flaky(max_runs=30, min_passes=1)
 def test_create_dataset_view(svc_client_with_repo):
     """Create a new dataset successfully."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
 
     payload = {
         'project_id': project_id,
@@ -74,7 +74,7 @@ def test_create_dataset_view(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_create_dataset_commit_msg(svc_client_with_repo):
     """Create a new dataset successfully with custom commit message."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
 
     payload = {
         'project_id': project_id,
@@ -100,7 +100,7 @@ def test_create_dataset_commit_msg(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_create_dataset_view_dataset_exists(svc_client_with_repo):
     """Create a new dataset which already exists."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
 
     payload = {
         'project_id': project_id,
@@ -125,7 +125,7 @@ def test_create_dataset_view_dataset_exists(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_create_dataset_view_unknown_param(svc_client_with_repo):
     """Create new dataset by specifying unknown parameters."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
 
     payload = {
         'project_id': project_id,
@@ -151,7 +151,7 @@ def test_create_dataset_view_unknown_param(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_create_dataset_with_no_identity(svc_client_with_repo):
     """Create a new dataset with no identification provided."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
 
     payload = {
         'project_id': project_id,
@@ -180,7 +180,7 @@ def test_create_dataset_with_no_identity(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_add_file_view_with_no_identity(svc_client_with_repo):
     """Check identity error raise in dataset add."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     payload = {
         'project_id': project_id,
         'dataset_name': 'mydata',
@@ -207,7 +207,7 @@ def test_add_file_view_with_no_identity(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_add_file_view(svc_client_with_repo):
     """Check adding of uploaded file to dataset."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     content_type = headers.pop('Content-Type')
 
     response = svc_client.post(
@@ -257,7 +257,7 @@ def test_add_file_view(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_add_file_commit_msg(svc_client_with_repo):
     """Check adding of uploaded file to dataset with custom commit message."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     content_type = headers.pop('Content-Type')
 
     response = svc_client.post(
@@ -301,7 +301,7 @@ def test_add_file_commit_msg(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_add_file_failure(svc_client_with_repo):
     """Check adding of uploaded file to dataset with non-existing file."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     content_type = headers.pop('Content-Type')
 
     response = svc_client.post(
@@ -344,7 +344,7 @@ def test_add_file_failure(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_list_datasets_view(svc_client_with_repo):
     """Check listing of existing datasets."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
 
     params = {
         'project_id': project_id,
@@ -370,7 +370,7 @@ def test_list_datasets_view(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_list_datasets_view_no_auth(svc_client_with_repo):
     """Check listing of existing datasets with no auth."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
 
     params = {
         'project_id': project_id,
@@ -390,7 +390,7 @@ def test_list_datasets_view_no_auth(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_create_and_list_datasets_view(svc_client_with_repo):
     """Create and list created dataset."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
 
     payload = {
         'project_id': project_id,
@@ -437,7 +437,7 @@ def test_create_and_list_datasets_view(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_list_dataset_files(svc_client_with_repo):
     """Check listing of dataset files"""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     content_type = headers.pop('Content-Type')
 
     file_name = '{0}'.format(uuid.uuid4().hex)
@@ -505,7 +505,7 @@ def test_list_dataset_files(svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_add_with_unpacked_archive(datapack_zip, svc_client_with_repo):
     """Upload archive and add it to a dataset."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     content_type = headers.pop('Content-Type')
 
     response = svc_client.post(
@@ -602,7 +602,7 @@ def test_add_with_unpacked_archive(datapack_zip, svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_add_with_unpacked_archive_all(datapack_zip, svc_client_with_repo):
     """Upload archive and add its contents to a dataset."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     content_type = headers.pop('Content-Type')
 
     response = svc_client.post(
@@ -702,7 +702,7 @@ def test_add_with_unpacked_archive_all(datapack_zip, svc_client_with_repo):
 @flaky(max_runs=30, min_passes=1)
 def test_add_existing_file(svc_client_with_repo):
     """Upload archive and add it to a dataset."""
-    svc_client, headers, project_id = svc_client_with_repo
+    svc_client, headers, project_id, _ = svc_client_with_repo
     payload = {
         'project_id': project_id,
         'dataset_name': '{0}'.format(uuid.uuid4().hex),