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

[dicomTar.pl] Exit when insertion or update fails #464

Merged

Conversation

cmadjar
Copy link
Collaborator

@cmadjar cmadjar commented Jul 17, 2019

Description

dicomTar.pl continues executing insertion into the tarchive tables even if one insertion failed. In this PR, we catch the insertion failure and return the error so dicomTar.pl can be stopped and further insertions prevented.

Needed to modified the database function of DCMSUM.pm to return the error when an SQL insertion failed during the execution of dicomTar.pl.

Note: ideally, the returned error message would be inserted in the notification_spool table which should be a separate PR.

This resolves

https://redmine.cbrain.mcgill.ca/issues/13841

…an SQL insertion failed during the execution of dicomTar.pl
Copy link
Contributor

@ycAbout ycAbout 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 looked the code, is there any guidelines to test them to see whether they work or not?

@cmadjar
Copy link
Collaborator Author

cmadjar commented Aug 7, 2019

Hi Corey,

Thank you for reviewing and testing.

Maybe you could try modifying the tarchive table in your sandbox (drop or rename a few columns) so that the insertion in the tarchive table fails and see if the pipeline stops and prints the correct error (in this case, that the query to insert into the tarchive table failed).

Then you could restore the tarchive table and modify the tarchive_series table and rerun dicomTar.pl. This time, you should see that the query to insert into tarchive_series failed.

Let me know if you have any question.

Copy link
Contributor

@ycAbout ycAbout left a comment

Choose a reason for hiding this comment

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

I have tried to upload scan, but cannot successfully run dicomTar.pl. But I figured out my way to run imaging_upload_file.pl, which includes dicomTar.pl. It worked. @cmadjar

@cmadjar cmadjar merged commit 442860c into aces:minor Aug 15, 2019
@cmadjar cmadjar deleted the exit_dicomTar_when_insertion_or_update_fails branch August 15, 2019 17:28
@cmadjar cmadjar modified the milestones: 21.1, 22.0 Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants