Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3353 batch job import #3497

Merged
merged 39 commits into from
Feb 9, 2017
Merged

3353 batch job import #3497

merged 39 commits into from
Feb 9, 2017

Conversation

bmckinney
Copy link
Contributor

@bmckinney bmckinney commented Nov 29, 2016

RFI Checklist

Before submitting the pull request, fill out sections (1.) Related Issues and (2.) Pull Request Checklist.

1. Related Issues

List and link to the issues in this Pull Request.


2. Pull Request Checklist

  • Functionality completed as described in FRD
  • Dependencies, risks, assumptions in FRD addressed
  • Unit tests completed
  • Deployment requirements identified (e.g., SQL scripts, indexing)
  • Documentation completed
  • All code checkins completed

3. Review Checklist

After the pull request has been submitted, fill out this section.

  • Code review completed or waived
  • Testing requirements completed
  • Usability testing completed or waived
  • Support testing completed or waived
  • Merged with develop branch and resolved conflicts

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 8.613% when pulling a99f929 on 3353-batch-job-import into 0cefe11 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 8.612% when pulling 4838dbd on 3353-batch-job-import into f739a75 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 8.613% when pulling 45a90ac on 3353-batch-job-import into f739a75 on develop.

…o override checksumType and checksumManifest; API returns NOT_IMPLEMENTED error if mode isn't MERGE; adds a dataset lock during import; adds integration test for DataverseRole.EDITOR
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 8.606% when pulling 9de9b24 on 3353-batch-job-import into f739a75 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 8.606% when pulling 16caeb2 on 3353-batch-job-import into f739a75 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 8.606% when pulling de8a649 on 3353-batch-job-import into f739a75 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 8.606% when pulling 993ec9e on 3353-batch-job-import into 75a4146 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 8.599% when pulling 616c12b on 3353-batch-job-import into 75a4146 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 8.598% when pulling c9402b9 on 3353-batch-job-import into 7591e36 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 8.598% when pulling c9402b9 on 3353-batch-job-import into 7591e36 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 8.59% when pulling e7e65d5 on 3353-batch-job-import into 7591e36 on develop.

…dle both the "package file" imports, and

imports of files as individual datafiles, as was originally developed.
@landreev
Copy link
Contributor

landreev commented Feb 2, 2017

OK, we agreed to make a number of changes during the last code review, 5 total, addressed in my latest commit:

  1. Reversed to the logic of trusting the external file copier to create the top-level folder for the files in the destination dataset directory. No extra folder is being created now.
    However, the final folder gets renamed, to a standard Dataverse-generated name, as used for "normal" files.
  2. Added an optional size parameter ("totalSize") to the batch job API call. If supplied, the long integer will be used as the size for the package Datafile. If not supplied, the import job will calculate the sum of all the files in the folder.
  3. Instead of calculating the checksum on the new line-separated list of individual cheksums, the checksum of the manifest file is used for the final datafile checksum.
  4. The functionality for the "individual file mode" (importing individual files as separate Datafiles) is preserved in the FileRecordWriter, but disabled on the API level; i.e., not available to the API user, for now.
  5. The mime type for a "file package" datafile is changed to a classier "application/vnd.edu.harvard.iq.dataverse.file-package".
    A boolean method isFilePackage() added to DataFile.java.

Please review and see if anything's missing.

Also, Phil just found something else that I missed - Bill created some RestAssured tests that are failing, now that the import process has been changed. Let's discuss how to proceed; should we move it to QA and treat the RA tests as a separate issue? Or fix it now?

Also, the branch needs to be synced up to the develop, before it can be QAed.

Resolved Conflicts:
	src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 9.387% when pulling 7d8b467 on 3353-batch-job-import into 1ba73ed on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 9.532% when pulling b1be2dc on 3353-batch-job-import into 1ba73ed on develop.

fixes the "multiple passes problem" - size, etc.
per the final conversation with Pete:
a new parameter - "uploadFolder";
the manifest goes back to specifying the files by the relative path inside the upload folder;
the manifest should be placed inside the upload folder.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 9.602% when pulling a1c46ef on 3353-batch-job-import into 1ba73ed on develop.

@kcondon kcondon merged commit 7162f5a into develop Feb 9, 2017
@kcondon kcondon deleted the 3353-batch-job-import branch February 9, 2017 16:15
@pdurbin pdurbin added this to the 4.6.1 - ORCID and File Replace milestone Mar 9, 2017
@pdurbin pdurbin mentioned this pull request May 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants