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

Transition from environment variables to parameter/configuration variables #7

Open
paulmking opened this issue Jul 5, 2018 · 5 comments
Assignees

Comments

@paulmking
Copy link
Collaborator

We should look at the list of environment variables we had used, and instead create parameter/configuration variables for things like raw data directory, rootfile output directory, etc.
Decide if it is useful to keep any of the environment variables.

Then either modify or remove the scripts in SetupFiles as appropriate.

It looks like the variables used from within the exisiting JAPAn are: QW_DATA, QW_ROOTFILES, QW_PRMINPUT, QW_TMP. The variables QW_FIELDMAP, QW_LOOKUP, and QW_SEARCHTREE were used by the QwTracking analysis, and so are not needed.

@paulmking
Copy link
Collaborator Author

Pull request #112 has specifically addressed the use of QW_DATA, but it likely also allows us to use configuration flags for all of the environment variables, in addition to having the environment variables.

I'll check the functionality of the other variables later today.

@wdconinc
Copy link
Member

wdconinc commented May 8, 2019

  • Implemented options -d/--data for $QW_DATA support #112 explicitly excludes support for all existing common QW_ environment variables except QW_DATA, per line
    } qw_name_mapper("QW_", "bin fieldmap lib lookup prminput rootfiles searchtree tmp");

    The reason for this approach is that the environment variable parsing will trip over QW_ environment variables that exist but aren't handled (similar to how boost::program_options trips over --option flags that are not handled, unless it is explicitly told not to consider them, an option that is not available for environment variables). Will someone accidentally define a QW_ variable? Maybe. The current approach of using an exclusion list was the quickest way that would allow expansive use for all options.
  • Since options are best defined and used in a single class, there are some issues with QW_TMP which is used in unrelated contexts. This would require some kind of inter-class communications (ugh).
  • QW_ROOTFILES support can easily be implemented, as can QW_ROOTFILES_LOCAL (if it will be retained).
  • QW_PRMINPUT is the tough one. There is a potential circular dependency since --add-config uses the QwParameterFile search path which would need QW_PRMINPUT. This would mean that QW_PRMINPUT would need to be simultaneously defined, parsed and processed first, but since QwOptions depends on it that's also not possible.

@wdconinc
Copy link
Member

wdconinc commented May 8, 2019

QW_TMP could probably be handled in a clever way as a QwOptions::GetTmpDir() which could be static (and QwOptions a singleton, which it isn't now if I recall).

@wdconinc
Copy link
Member

wdconinc commented May 8, 2019

QwOptions is essentially already a singleton (since we interact with it through the global gQwOptions). Except we don't force it to be. Might be better to just implement it as a real singleton.

@wdconinc
Copy link
Member

wdconinc commented May 9, 2019

Related: #115.

That presents a solution for QW_TMP too, just replacing the getenv call with gQwOptions.GetValue<string>("tmp").

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

No branches or pull requests

2 participants