Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: ✨Add jinja settings support for golden config plugin #527
feat: ✨Add jinja settings support for golden config plugin #527
Changes from 5 commits
d3941b8
444b7fa
47d96d0
c0287ac
d655eea
bfa531b
a450d6c
5162ea9
7c75303
1f201ac
6710c5a
a460597
f35ef16
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
After getting internal feedback, I think the point @jeffkala and @electro47h is that there are many variables one could set for Jinja, I think the list is:
It probably makes sense to support them, as a dictionary, with the 3 variables you have set as defaults. So, there should likely be one variable called
jinja_env
that is set. If you want to have a magic method of some sort such that anything set withjinja_env_*
automagically gets into the dictionary, I could buy that as well, as long as it is in addition to the dictionary method.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.
Understood. None of those were possible to set before, but since they will be now, they should be included. This will take a good bit of rework on my end. I'm going to draft this PR and work on it later this week if time allows.
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.
Question for the masses here. For an option such as 'undefined', how would that look in the nautobot_config.py file for a setting?
It appears most of the settings are boolean or string values, but a few of the options listed for Jinja are enumerations or classes. If the user intended to customize that setting in their nautobot_config.py file, would they have to add the import, or is there another way to represent that?
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.
Yes, they would have to add that to the import within nautobot_config.py. We could work around it, but the complexity is not really worth it.
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.
Thank you. I have committed what my interpretation of the request is.
I'm out of the office tomorrow and will be unable to test. We also have to get our environment upgraded from 1.5.24 to >=1.6.1 to install this version of the plugin. I will work to get this tested when I can.
Please let me know if this is the correct architecture that you're looking for, or if I need to make changes. Happy to keep plugging away at this.