search

ec-jrc / pyPoseidon

Framework for Hydrodynamic simulations
https://pyposeidon.readthedocs.io/
European Union Public License 1.2
20 stars 7 forks source link

launchSchism.sh: Handle ABORT in bash and consolidate execution of launchSchism.sh #156

Closed pmav99 closed 1 year ago

pmav99 commented 1 year ago

Fixes #155

pmav99 commented 1 year ago

@brey with this PR I transfer the check for ABORT within launchSchism.sh. The practical implication of this change is that launchSchism.sh now always returns a proper error code. So if ABORT is in STDOUT/STDERR, then we always get an error code, regardless of the return code of the schism process. Nevertheless, this introduces a small difference in the API.

Previously, in schism.py we were storing proc.stdout and proc.stderr as instance attributes, regardless of what happened with the execution. https://github.com/ec-jrc/pyPoseidon/blob/17b3a664759f9ae4d7f0addd1f788a13cf3d8ac2/pyposeidon/schism.py#L709-L711

With the PR we only store them if and only if the execution was successful. I.e. we raise a ProcessCallError before we have the chance to save the attributes.

it is possible to revert to the old behavior, but the code is kind of ugly and I don't think that it is worth it. Check it out and let me know what you think.

Other than that I don't think that there is any difference.

brey commented 1 year ago

I don't think there is a problem. Saving the output to the instance was introduced a few weeks ago and I believe we shouldn't even do that. SCHISM has its own log for errors (fatal.error, nonfatal_*, mirror.out). Let's drop it.

pmav99 commented 1 year ago

Is the launchSchism.sh code OK with you? Is it clear does it need any clarification?