-
Notifications
You must be signed in to change notification settings - Fork 4
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
#810 Add the possibility to zip the inputs and outputs for N simulation #820
base: master
Are you sure you want to change the base?
#810 Add the possibility to zip the inputs and outputs for N simulation #820
Conversation
cc022b3
to
abe282a
Compare
Signed-off-by: Dimitri Baron <dimitri.baron@rte-france.com>
abe282a
to
1826950
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the N output will not be zipped. Also, the documentation and tests needs to be updated.
"$MPIRUN_PATH" -np $NBPROCS $DYNAFLOW_LAUNCHER_INSTALL_DIR/bin/DynaFlowLauncher --log-level $DYNAFLOW_LAUNCHER_LOG_LEVEL \ | ||
--network $2 \ | ||
--config $3 \ | ||
--contingencies $4 | ||
fi | ||
else | ||
if [ -n "$ZIP_ARCHIVE" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should add a check that the zip archive exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check is made in the C++ code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but it is also the case for network, config and contingencies files and yet they are tested in the other branch. This is to be homogeneous
Signed-off-by: Dimitri Baron <dimitri.baron@rte-france.com>
|
||
|
||
std::string formatZipErrorMessage(const zip::ZipException& e) { | ||
std::stringstream zipErrorStream; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why overriding the messages sent by libzip? The general exception catch should be enough ?
Checklist before requesting a review
use
'[x]'
to check the checkboxes, or submit the PR and then click the checkboxes