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.

Sign up for GitHub

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

Jump to bottom

3942 dcm notifications #4034

Merged
merged 33 commits into from
Aug 10, 2017
Merged

3942 dcm notifications #4034

merged 33 commits into from
Aug 10, 2017

Conversation

pdurbin
Copy link
Member

@pdurbin pdurbin commented Aug 1, 2017

Related Issues

Pull Request Checklist

sekmiller and others added 18 commits July 27, 2017 09:26
@sekmiller
#3942 Add DCM Notifications for CheckSum failure
e2927ef
@sekmiller
Merge branch 'develop' into 3942-DCM-notifications
9a3f856
@sekmiller
#3942 Add Notifications Test
94fb8b9 
Fix double entries
@sekmiller
Merge branch 'develop' into 3942-DCM-notifications
150b651
@sekmiller
#3942 - add note on fix of duplicates
fa3520f 
Also remove debug code
@pdurbin
kick off file system import job on success #3942
7681c63
@pdurbin
Merge branch 'develop' into 3942-DCM-notifications #3942
2ac7ed4
@sekmiller
#3942 Move FindDataset to AbstractAPI
54233e5
@pdurbin
prepare for extra optional testing #3942
e93ab6f
@sekmiller
#3942 - update comment
b5f5f95
@pdurbin
make receiveChecksumValidationResults superuser only #3942
536632f
@pdurbin
the meaning of datasetId has changed from globalId to id #3942
19ed1af
@sekmiller
#3942 remove/update debug code
ef5b80d
@pdurbin
refactor into UtilIT.dataCaptureModuleChecksumValidation #3942
02aa3f7
@pdurbin
assert operations on dataset id and persistentId work #3942
42d7237
@pdurbin
rename DCM page to Big Data Support in Installation Guide #3942
949c7e4
@pdurbin
document how DCM passes checksum validation results #3942
9aef09b
@pdurbin
document low level "import" API endpoint #3942
39cc6b6
@pdurbin pdurbin mentioned this pull request Aug 1, 2017
Closed
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 10.733% when pulling 39cc6b6 on 3942-DCM-notifications into e6a8b99 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 10.733% when pulling 3af2355 on 3942-DCM-notifications into e6a8b99 on develop.

@pdurbin
Copy link
Member Author

pdurbin commented Aug 1, 2017 •
edited by pameyer
Loading

Here's the todo list/punch list from the meeting we just had:

img_20170801_154106

Also, @pameyer supplied existing messages:

happy path:
Dataset doi:XXXX/XXXX/XXX has been successfully uploaded and verified. If you have any questions, please contact XXXX@XXXX.XXX.

failure path:
One or more files in your dataset failed to match the checksum calculated on your local system. Please re-run the upload script associated with this dataset. If you have any questions, please contact XXXX@XXXX.XXX.

oscardssmith
oscardssmith reviewed Aug 2, 2017
View reviewed changes
src/main/java/edu/harvard/iq/dataverse/api/Datasets.java Outdated

@POST
@Path("{identifier}/dataCaptureModule/checksumValidation")
public Response receiveChecksumValidationResults(@PathParam("identifier") String id, JsonObject jsonFromDcm) throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where does this throw?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops. I'll take that out as well as some imports I added when adding test code. Good catch, @oscardssmith . Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in f3048e0.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 11.369% when pulling 3b263e2 on 3942-DCM-notifications into e6a8b99 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 11.369% when pulling 8a2bb41 on 3942-DCM-notifications into e6a8b99 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 11.369% when pulling e812470 on 3942-DCM-notifications into e6a8b99 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 11.381% when pulling f3048e0 on 3942-DCM-notifications into e6a8b99 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 11.381% when pulling f3048e0 on 3942-DCM-notifications into e6a8b99 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 11.381% when pulling 2f394b9 on 3942-DCM-notifications into f23ed5f on develop.

@pameyer pameyer self-assigned this Aug 2, 2017
pameyer
pameyer reviewed Aug 2, 2017
View reviewed changes
src/main/java/edu/harvard/iq/dataverse/api/Datasets.java Outdated
if ("validation passed".equals(statusMessageFromDcm)) {
try {
// String url = "http://localhost:8080/api/batch/jobs/import/datasets/files/${DOI_SHOULDER}/${datasetIdentifier}?mode=MERGE&uploadFolder=trn&totalSize=${sz}&userId=${dv_userId}";
String url = systemConfig.getDataverseSiteUrl() + "/api/batch/jobs/import/datasets/files/" + dataset.getId();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not leave this as calling localhost?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pameyer no objection. Fixed in 6c9c249.

@pameyer pameyer removed their assignment Aug 2, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 11.381% when pulling bbfd71a on 3942-DCM-notifications into e859001 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 11.378% when pulling 2dd89a5 on 3942-DCM-notifications into e859001 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 11.378% when pulling 6c9c249 on 3942-DCM-notifications into e859001 on develop.

matthew-a-dunlap
matthew-a-dunlap reviewed Aug 3, 2017
View reviewed changes
Copy link
Contributor

@matthew-a-dunlap matthew-a-dunlap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've only read over the sphinx guide, but it reads ok to me!

@pdurbin
remove null check (caught by WrappedResponse) #3942
3164bb6 
That's why it's called `findDatasetOrDie`.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 11.364% when pulling 3164bb6 on 3942-DCM-notifications into e859001 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 11.36% when pulling f8809c3 on 3942-DCM-notifications into e859001 on develop.

@pdurbin
cleanup move now unused methods to test class #3942
59120c4 
The methods aren't used anymore by the code but they are still useful
for testing.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 11.364% when pulling 59120c4 on 3942-DCM-notifications into e859001 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 11.363% when pulling 87241de on 3942-DCM-notifications into e859001 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 12.058% when pulling 103b40c on 3942-DCM-notifications into a1792ad on develop.

@kcondon kcondon merged commit dd55c08 into develop Aug 10, 2017
@kcondon kcondon deleted the 3942-DCM-notifications branch August 10, 2017 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthew-a-dunlap matthew-a-dunlap matthew-a-dunlap left review comments

@pameyer pameyer pameyer left review comments

@oscardssmith oscardssmith oscardssmith left review comments

@michbarsinai michbarsinai michbarsinai approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.8 - AWS S3 Support and Large Data U...
Development

Successfully merging this pull request may close these issues.
8 participants
@pdurbin @coveralls @michbarsinai @matthew-a-dunlap @pameyer @oscardssmith @kcondon @sekmiller