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

Fix for Remine ticket #10927: the server processes files are deleted only if the pipeline's exit code is 0. #2252

Merged
merged 2 commits into from
Oct 14, 2016
Merged

Fix for Remine ticket #10927: the server processes files are deleted only if the pipeline's exit code is 0. #2252

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be97ddd
Fix for Remine ticket #10927: the server processes files are deleted …
nicolasbrossard Oct 4, 2016
99f7582
PR was missing the declaration of variable $exitCode
nicolasbrossard Oct 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions modules/server_processes_manager/php/MriUploadServerProcess.class.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,20 @@ class MriUploadServerProcess extends AbstractServerProcess
{
return self::PROCESS_TYPE;
}

/**
* Whether $outfile, $_errfile and $_exitCodeFile should be deleted once the
* process is finished. These files will be deleted only if the pipeline
* executed successfully (i.e. with exit code 0).
*
* @return boolean true if the files should be deleted, false otherwise
*/
public function deleteProcessFiles()
{
$exitCode = parent::getExitCode();

return !is_null($exitCode) && is_numeric($exitCode) && $exitCode == 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

$exitCode doesn't seem to be defined in any scope that would mean the variable could be used here like this. It's neither an argument nor local variable..

Copy link
Contributor

@MounaSafiHarab MounaSafiHarab Oct 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@nicolasbrossard @driusan
shouldn't the change have been made in:
https://github.com/aces/Loris/blob/master/modules/server_processes_manager/php/AbstractServerProcess.class.inc#L521
This is the line that I used to alternate between True/False depending on whether I wanted to keep those files or not...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleting the process files only if the exit code is 0 is something that is specific to the MRI upload processes. I think the default behaviour (to delete all files all the time, which is defined in AbstractServerProcess) should remain as is and should only be overridden in MriUploadServerProcess.

}
}

?>