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

ShellJob: Detect and prevent filename clashes #70

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

sphuber
Copy link
Owner

@sphuber sphuber commented Feb 10, 2024

Fixes #68

So far, no validation was performed on the filenames of input nodes that
would be written to the working directory that could overlap with files
written by the plugin itself, such as stdout and stderr. This could
lead to unexpected results and often calculations failing completely.

For example, if a SinglefileData would be written to the working
directory with the filename stdout, most likely because the filename
attribute of the node was set to this, then this would be overwritten by
the redirection of the stdout, which is redirected to the file stdout
by default.

A distinction is made between clashes that happen "implicitly", meaning
the filename of the SinglefileData or FolderData just happen to
overlap, compared to "explicitly" where the user specifies a conflicting
filename in the filenames input. In the latter case, an exception is
raised during the validation. In the former case, the target filename is
automatically changed to a unique filename to resolve the naming issue.
This solution is chosen because this will happen often in normal use
cases of aiida-shell. For example, if a command is run whose stdout is
captured, it will be stored as a SinglefileData with the name
stdout. If this in turn is used as an input for a next command, this
filename will clash as stdout is a reserved name. This would force the
user to manually correct the problem by defining an alternative name in
filenames. Since this will occur so often, it is better to have the
plugin automatically solve this conflict.

There is a special case for the FolderData where an object contained
within it overlaps with a reserved filename. In this case, the strategy
mentioned above of automatically renaming won't work because the writing
of the contents is not done by the plugin but by the engine. Instead, an
exception is raised. Unfortunately, this is not done in the input
validation and so this will result in an excepted process. However, it
is currently not possible to put this logic in the input validation as
this would require significant refactoring.

@sphuber sphuber force-pushed the fix/068/input-output-filename-overlap branch 2 times, most recently from 237a37c to d38d379 Compare February 10, 2024 19:05
So far, no validation was performed on the filenames of input nodes that
would be written to the working directory that could overlap with files
written by the plugin itself, such as `stdout` and `stderr`. This could
lead to unexpected results and often calculations failing completely.

For example, if a `SinglefileData` would be written to the working
directory with the filename `stdout`, most likely because the `filename`
attribute of the node was set to this, then this would be overwritten by
the redirection of the stdout, which is redirected to the file `stdout`
by default.

A distinction is made between clashes that happen "implicitly", meaning
the filename of the `SinglefileData` or `FolderData` just happen to
overlap, compared to "explicitly" where the user specifies a conflicting
filename in the `filenames` input. In the latter case, an exception is
raised during the validation. In the former case, the target filename is
automatically changed to a unique filename to resolve the naming issue.
This solution is chosen because this will happen often in normal use
cases of `aiida-shell`. For example, if a command is run whose stdout is
captured, it will be stored as a `SinglefileData` with the name
`stdout`. If this in turn is used as an input for a next command, this
filename will clash as `stdout` is a reserved name. This would force the
user to manually correct the problem by defining an alternative name in
`filenames`. Since this will occur so often, it is better to have the
plugin automatically solve this conflict.

There is a special case for the `FolderData` where an object contained
within it overlaps with a reserved filename. In this case, the strategy
mentioned above of automatically renaming won't work because the writing
of the contents is not done by the plugin but by the engine. Instead, an
exception is raised. Unfortunately, this is not done in the input
validation and so this will result in an excepted process. However, it
is currently not possible to put this logic in the input validation as
this would require significant refactoring.
@sphuber sphuber force-pushed the fix/068/input-output-filename-overlap branch from d38d379 to 415b27e Compare February 19, 2024 10:11
@sphuber sphuber merged commit 043ed84 into master Feb 20, 2024
9 checks passed
@sphuber sphuber deleted the fix/068/input-output-filename-overlap branch February 20, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a check that input files are not overwritten by output files
1 participant