-
Notifications
You must be signed in to change notification settings - Fork 300
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
DRAFT: yamlinclude initial commit #1038
base: main
Are you sure you want to change the base?
Conversation
|
||
with open(path, "r") as file: | ||
try: | ||
yfile = yaml.safe_load(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.
Limit the try block to just this line
@@ -0,0 +1,96 @@ | |||
import os.path |
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 recommend running black
on this file
# | ||
# We parse that into a dictionary for lookup later as part of the substitution loop. | ||
subs = {} | ||
if ('subs') in self.options: |
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 need for parenthesis -- makes it look at first glance like you might be creating a tuple
# We parse that into a dictionary for lookup later as part of the substitution loop. | ||
subs = {} | ||
if ('subs') in self.options: | ||
for i in self.options.get('subs').splitlines(): |
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.
Just self.options["subs"]
is fine. get()
is unnecessary
|
||
if ('debug') in self.options: print ("Substitution dictionary is "); print(subs) | ||
|
||
key = self.options.get('key') |
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.
You probably want self.options["key"]
unless you actually want to handle the case where key
doesn't exist
This just has the basic YAMLINCLUDE logic.
I have to figure out how best to use this. The place it might make the most sense is the absurdly long
common-mc-admin-config.rst
, but before doing that I want to let #1028 complete.So for now will let this sit and think about whether this actually adds value to our workflow.