Skip to content
This repository has been archived by the owner on Oct 11, 2023. It is now read-only.

$SIMDEM_TEMP_DIR and SIMDEM_VERSION not set in SimDem2 #104

Closed
SorraTheOrc opened this issue Mar 12, 2018 · 8 comments
Closed

$SIMDEM_TEMP_DIR and SIMDEM_VERSION not set in SimDem2 #104

SorraTheOrc opened this issue Mar 12, 2018 · 8 comments

Comments

@SorraTheOrc
Copy link
Contributor

In SimDem1 we provided default SIMDEM_TEMP_DIR environment variable, this is used as a workspace when SimDem or a SimDem script needs to write some temporary files. By default it is set to [.simdem/tmp](https://github.com/Azure/simdem/blob/6dbce90c0bfbd980e35cdc3c92edd28e1e612ffe/config.py#L2)

Similarly we set SIMDEM_VERSION for easy validation of the version against expected versions during testing. This could be replaced by a --version switch in the CLI.

SimDem2 does not provide either of these and so the test scripts for SimDem1 fail:

$ ~/.local/bin/simdem --mode test README.md
$ ls $SIMDEM_TEMP_DIR/test
ls: cannot access '/test': No such file or directory

***PREREQUISITE VALIDATION FAILED***
$ touch $SIMDEM_TEMP_DIR/test/nested_prereq_ran
touch: cannot touch '/test/nested_prereq_ran': No such file or directory

$ ls $SIMDEM_TEMP_DIR/test
ls: cannot access '/test': No such file or directory

***PREREQUISITE VALIDATION FAILED***
$ touch $SIMDEM_TEMP_DIR/test/prereq_ran
touch: cannot touch '/test/prereq_ran': No such file or directory

$ ls $SIMDEM_TEMP_DIR/test
ls: cannot access '/test': No such file or directory

***PREREQUISITE VALIDATION FAILED***
$ touch $SIMDEM_TEMP_DIR/test/nested_prereq_ran
touch: cannot touch '/test/nested_prereq_ran': No such file or directory

$ ls $SIMDEM_TEMP_DIR/test
ls: cannot access '/test': No such file or directory

***PREREQUISITE VALIDATION FAILED***
$ touch $SIMDEM_TEMP_DIR/test/prereq_ran
touch: cannot touch '/test/prereq_ran': No such file or directory

$ echo $SIMDEM_VERSION


*** SIMDEM TEST RESULT FAILED ***
@lastcoolnameleft
Copy link
Contributor

I've added --version flag in this commit: c364431

➜  simdem git:(simdem2) simdem --version
simdem 0.9.0

Does this meet the $SIMDEM_VERSION requirement, or did you still want the env variable injected?

@SorraTheOrc
Copy link
Contributor Author

To be honest I can't for the life of me remember why there is an environment variable for VERSION, so I guess that means we can drop it.

The TEMP_DIR is important though, many of the scripts already out there use it. Happy for this to simply be a default that can be overridden with the '-e' argument. I keep dreaming of a .simdem/config file to configure these kinds of things but we should not block on that.

@lastcoolnameleft
Copy link
Contributor

Regarding the directory location, it's easy to put into the config file (which is overridable); however, the key part is what is it relative to?

  • The user's home directory?
    • My preference because it would be consistent across runs
  • Where the CLI was run?
    • This could break with a "Next Step" in a different directory.
  • Where the current script is run?
    • This could break with a "Next Step" in a different directory.

@SorraTheOrc
Copy link
Contributor Author

SorraTheOrc commented Mar 16, 2018 via email

@lastcoolnameleft
Copy link
Contributor

This is now implemented to use the value here: https://github.com/Azure/simdem/blob/simdem2/simdem/simdem.ini#L5

@SorraTheOrc
Copy link
Contributor Author

The variable name is defined as "temp_dir" rather than "SIMDEM_TEMP_DIR". Is there a reason for this? I'd much rather we stay backward compatible because:

  • no changes to existing scripts (including tests)
  • the "SIMDEM" prefix makes it possible to have a setting in the environment without interfering with other applications
  • it's convention for environment variables to be all upper case

@SorraTheOrc SorraTheOrc reopened this Mar 19, 2018
@lastcoolnameleft
Copy link
Contributor

"temp_dir" is what it's called inside the simdem.ini file it gets mapped to SIMDEM_TEMP_DIR inside the coe. I can change the name if you want me to, but I figured it would be useful to keep it consistent to *.ini settings.

Here is where it's set to SIMDEM_TEMP_DIR:
https://github.com/Azure/simdem/blob/simdem2/simdem/mode/common.py#L138

@SorraTheOrc
Copy link
Contributor Author

Ahhh.. OK. That's confusing, but it's only a documentation thing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants