-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Use envvars for configuration instead of a file #1091
Conversation
This seems to improve the time it takes to start tools (and this should be reflected on Travis test time too): before: $ time ./tools/inference.py --help
real 0m0.821s After: $ time ./tools/inference.py --help
real 0m0.438s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we document the full list of environment variables?
@classmethod | ||
def get_extensions(cls, show_all): | ||
return extensions.view.get_extensions(show_all=show_all) | ||
if 'DIGITS_ALL_EXTENSIONS' in os.environ: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice simplification!
# make the directory | ||
os.mkdir(self._config_file_value) | ||
|
||
if 'DIGITS_MODE_TEST' in os.environ: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe move this to a function like you are doing for other options like log_file
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really think that's more legible? I thought I'd just keep it simple when I can.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I think it's fine this way, forget that comment.
if config_value('caffe_root')['flavor'] == 'NVIDIA': | ||
if config_value('caffe_root')['version'] > parse_version('0.14.0-alpha'): | ||
if config_value('caffe')['flavor'] == 'NVIDIA': | ||
if parse_version(config_value('caffe')['version']) > parse_version('0.14.0-alpha'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested this path - ok
|
||
|
||
if 'CAFFE_ROOT' in os.environ: | ||
executable, version, flavor = load_from_envvar('CAFFE_ROOT') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested this path - OK
raise ValueError('Torch executable not found at "%s" (TORCH_HOME)' | ||
% os.environ['TORCH_HOME']) | ||
else: | ||
executable = find_executable() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested this path - OK
I added a doc at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe edit the doc in https://github.com/NVIDIA/DIGITS/blob/v4.0.0/docs/BuildDigits.md#building-digits too?
Oops, I meant to squash before merging. Sorry. |
Use envvars for configuration instead of a file
Close #941, close #180, fix #1090
Bug 1804528
Also,