-
Notifications
You must be signed in to change notification settings - Fork 0
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
DM-48445: Replace ScriptTemplates with Jinja Template #160
DM-48445: Replace ScriptTemplates with Jinja Template #160
Conversation
036b0f0
to
3635692
Compare
f7b1ae0
to
4005184
Compare
3635692
to
b32f994
Compare
2b8d70c
to
9d696be
Compare
f0d5b44
to
88bfbf0
Compare
bc8beaa
to
adbae29
Compare
a6511af
to
a58184e
Compare
791f535
to
ddd6efd
Compare
a58184e
to
a5339f3
Compare
ddd6efd
to
1397c3f
Compare
7e95e3e
to
0eaceab
Compare
1397c3f
to
d36e6d3
Compare
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.
Looks great.
@@ -0,0 +1,24 @@ | |||
#!/usr/bin/env bash | |||
{%- if wms == "htcondor" %} |
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.
The only potential pitfall I see here is that wms
seems to come from some deep down plumbing, so I could imagine that going missing somehow and then the script fails in a very confusing way. I don't know if there's a good way to guard against that, since wms=None
might also be valid in some cases?
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.
For now I'll use the class attribute from the WmsMethodEnum which is otherwise unused. This will promote the value from being part of a "specblock" to being part of the actual Handler instance used for the script, so it should always be both available and set correctly.
e900a60
to
931a0c3
Compare
Use Jinja2 templates for rendering scripts Deprecate and remove use of ScriptTemplates
0eaceab
to
a8d6a26
Compare
931a0c3
to
b9cdf53
Compare
Removes ScriptTemplates from the application.
Does not deprecate or remove APIs related to ScriptTemplates, but eliminates the use of ScriptTemplates from both seed (example) files and script-generation functions and methods.
The concept of "script templates" is refactored using Jinja template files with "snippets" enabled or disabled based on configuration flags, e.g., "wms: htcondor" enables a template block that switches on the value of
wms
. In this way, a single template file can be considered, updated, and rendered according to the specific needs of a campaign instead of pulling in multiple template snippets from the database.This pattern is also used in the process of writing a bash script to submit to htcondor for execution, with a simplified approach to setting up a "condor" or a "panda" stack environment and without the extra steps concerning clobbering and reimplementing environment variables in the shebang.