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

Add support for Temporary directory #56

Open
jcfr opened this issue Feb 10, 2016 · 4 comments
Open

Add support for Temporary directory #56

jcfr opened this issue Feb 10, 2016 · 4 comments

Comments

@jcfr
Copy link
Member

jcfr commented Feb 10, 2016

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
Copy link
Member Author

jcfr commented Feb 10, 2016

Cc: @Slicer/slicer-core

@lassoan
Copy link
Contributor

lassoan commented Feb 10, 2016

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.

@jcfr
Copy link
Member Author

jcfr commented Feb 10, 2016

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.

@pieper
Copy link
Member

pieper commented Feb 10, 2016

+1, --temporaryDir sounds good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants