SlicerExecutionModel icon indicating copy to clipboard operation
SlicerExecutionModel copied to clipboard

Add support for Temporary directory

Open jcfr opened this issue 9 years ago • 4 comments

Discussion: http://slicer-devel.65872.n3.nabble.com/Temporary-directory-for-CLI-modules-td4035464.html

Reason to reject initial patch from @ prastawa : Patch Slicer/Slicer#400 will be problematic when the CLI is executed directly without Slicer.

Proposal:

  • CLI should always expect the environment variable SEM_TEMPORARY_DIR to be set
  • SEM_TEMPORARY_DIR can be set in three different ways:
    1. Set by reading the value of an other environment variable. This other environment variable would be set configuring SlicerExecutionModel using for example -DSEM_TEMPORARY_DIR_ENVVAR_NAME:STRING=SLICER_TEMPORARY_DIR
    2. Set by reading the value of special parameter named --sem-temporary-dir. If specified, other environment variable would be ignored
    3. If neither (1) or (2) have been set, temporary directory will be set to the operating system default
  • code in charge of reading the other environment variable, or special parameter would be embedded in the CLI thanks to code generated by GenerateCLP

jcfr avatar Feb 10 '16 04:02 jcfr

Cc: @Slicer/slicer-core

jcfr avatar Feb 10 '16 04:02 jcfr

Sounds good to me. Just two suggestions:

  1. Instead of --sem-temporary-dir we could simply use --temporaryDir. We already reserve special argument names without using any prefix (e.g., we use --logo instead of --semLogo). I would use temporaryDir instead of temporary-dir to be consistent with other command-line argument names (- is not used because C variable names cannot contain -).
  2. Why do we pass the temporary dir to the CLI main() function as an env var? PARSE_ARGS could simply store the temporary directory as a string, for example temporaryDir. It would be more consistent with how other values that are specified as a command-line arguments are passed on the the main() function.

lassoan avatar Feb 10 '16 04:02 lassoan

temporary dir to the CLI ... as a command-line arguments [...] using --temporaryDir

Only using PARSE_ARGS makes sense and would greatly simplify the initial proposal. If not specified, a reasonable default would be selected. (Operating system one, or current directory if not available)

Why do we pass the temporary dir to the CLI main() function as an env var?

My initial proposal was broad enough to include all the idea previously discussed. Going with --temporaryDir makes sense to me.

jcfr avatar Feb 10 '16 04:02 jcfr

+1, --temporaryDir sounds good to me

pieper avatar Feb 10 '16 11:02 pieper