-
Notifications
You must be signed in to change notification settings - Fork 735
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 Environment Variable Option for Custom Template Location #863
Add Environment Variable Option for Custom Template Location #863
Conversation
if not os.path.isdir(template_dir): | ||
project_dir = os.path.dirname(os.path.dirname(os.path.dirname(template_dir))) | ||
template_dir = os.path.join(project_dir, "templates") | ||
template_dir = os.environ.get("NTC_TEMPLATES_DIR") |
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.
I'm wondering if we should offer both an OS environment or pass in as an arg to parse_output
.
Something like.
def parse_output(platform=None, command=None, data=None, template_dir=None):
template_dir = os.environ.get("NTC_TEMPLATES_DIR") or template_dir
if template_dir is None:
template_dir = _get_template_dir()
And then keep _get_template_dir()
the same.
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.
I would say it is good to have _get_template_dir
do the environment lookup, but allow this function to override the environment by passing in the directory.
def parse_output(platform=None, command=None, data=None, template_dir=None):
template_dir = template_dir or _get_template_dir()
I'm also okay with the PR like it is since that is more than is available today and a reasonable solution
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.
@jmcgill298 @FragmentedPacket , just let me know either way. I don't mind taking some extra time to implement the ability to set an arg.
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.
I'm with @jmcgill298 and his way is better than mine.
I'll approve it and let you decide :)
ISSUE TYPE
COMPONENT
core update to
parse
to add ability to set custom template location via environment variable.SUMMARY
Allow for custom templates directory to be specified and used. This is helpful when you need to develop custom templates and usages within a project directory.
If the environment variable is not defined it reverts to existing logic to load from installed path. I have tested the change.