EmPy as alternative to Jinja2 #2734
Conversation
|
@TomekTrzeciak - I can imagine arguing that supporting multiple templating systems will make it harder to support Cylc users, but if this is better than Jinja2 for our purposes and it makes bleeding edge complex suites easier to write (and you make a convincing argument) then I think it's worth it. One question, EmPy looks pretty old (not that that's necessarily bad) - did you consider other options too, e.g. from here https://wiki.python.org/moin/Templating ? Presumably most suffer from similar issues to Jinja2 (for Cylc usage) but maybe some don't? Just curious... |
|
This isn't working for me, e.g.: Validation (etc.) prints out file contents and then fails: |
TomekTrzeciak
force-pushed
the
empy_support
branch
from
b09530e
to
2a60521
Compare
|
@hjoliver, sorry I think I made some changes that broke it and then forgot about them and pushed it. I fixed it and rebased it - should be working now. As for your earlier comment - I had the same dilemma that an additional templating system would be extra support burden, but in practice I don't think this will be that significant. The real support problem is that there is templating in Cylc, what templating system is used is probably secondary to that. Unfortunately, some form of templating is needed (or people would roll their own), but fortunately this part of Cylc is really well separated from the rest of the code base (which made adding EmPy so easy for me!). I did have a look at other templating systems, but they all seemed to me like a worse fit for Cylc than EmPy. I had the following criteria:
Mako came close to these requirements. In many respects it is quite similar to Jinja, but lets you use "straight Python blocks, inline or at the module-level". Unfortunately, the syntax is likely to clash with embedded shell scripts in Cylc (Mako uses shell/Perl inspired |
|
@TomekTrzeciak - works now, thanks. Note I've put up a PR to your branch with a vim syntax update, and to move your example into the correct |
|
@hjoliver How do we want to document this? I'm guessing we don't want to produce all of our code examples in both Jinja2 and EmPy. Given the current usage of Jinja2 it might be an idea to keep Jinja2 as the "dafacto" templating engine but add a short section documenting the support for EmPy. |
|
Yeah, minimal documentation of Empy is fine, along with some explanation of where or why it might be preferred over Jinja2. |
|
Many Codacy issues flagged here. Perhaps we don't need to bundle EmPy? It is very mature and stable after all. |
|
@hjoliver, I guess you don't run codacy for Jinja2 (it has many problems too), so the same should go for EmPy too, shouldn't it? |
|
@TomekTrzeciak - that's true, and we can certainly tell Codacy not to analyse EmPy. But the reason we bundle Jinja2 is (at least partly, as I recall) that it is still evolving fast enough that we've had occasional serious issues with different behaviour between versions. It's seems unlikely that we'll have those kinds of problems with EmPy. Let's see what the concensus is from @cylc/core ... |
|
I don't think we need to bundle EmPy, the interface isn't evolving and we've bundled quite enough already. We can add it as an optional dependency in |
|
I admit that I would prefer to bundle EmPy with Cylc. It's not really optional like the analytics in the sense that the suite won't even run without it. Otherwise, to make it developer-friendly, it will have to be bundled with every suite that uses it, because I don't expect to find it on any site installation. Also, I think it will be easier to make sure it keeps working if it can be included within the standard |
|
@TomekTrzeciak - I can appreciate your position, but we do have other "compulsory" dependencies that are not bundled (Jinja2 used to be one of them), and I have the impression that bundling is generally not considered a good thing unless you're forced into it by (e.g.) overly rapid changes in interface or behaviour that you depend on, or not easily available by standard installation methods. We can list EmPy in the software dependencies section and add it to |
|
@hjoliver, OK I'll unbundle python -c 'import em' 2>/dev/null || skip_all 'EmPy not available' |
|
@TomekTrzeciak - thanks. Yes, that ought to do it! |
|
Some tests failed, but I don't understand why. They pass for me when run locally. |
|
@TomekTrzeciak - I see you've |
|
Error try:
os.chdir(dir_)
ftempl = StringIO('\n'.join(flines))
xtempl = StringIO()
interpreter = em.Interpreter(output=em.UncloseableFile(xtempl))
interpreter.file(ftempl, '<template>', template_vars)
except Exception as exc:
lineno = interpreter.contexts[-1].identify()[1] # ??? not defined here ??? |
|
Printing the actual exception message (on Travis CI, with a branch modified to run just the failing tests) I get: |
|
Ah ha - an unfortunate package name clash! On my own ubuntu VM:
In the latter case,
|
|
(You should warn about this in the Cylc EmPy documentation!) |
lib/parsec/fileparse.py
Outdated
| if cylc.flags.verbose: | ||
| print "Processing with EmPy" | ||
|
|
||
| # import EmPy only when truly required (might throw ImportError) | ||
| from parsec.empysupport import EmPyError, empyprocess |
There was a problem hiding this comment.
We might want to provide an explanation with this traceback so that it doesn't look like an internal error to the uninformed user:
try:
from parsec.empysupport import EmPyError, empyprocess
except ImportError as exc:
exc.message = ('The empy Python module must be installed in '
'order to perform empy templating.\n%s' % exc.message)
raise excOr
raise ParsecError("he empy Python module...")
doc/src/cylc-user-guide/cug.tex
Outdated
| expressions, loop control structures, conditional logic, etc., that are expanded | ||
| to generate the final suite configuration seen by cylc. See | ||
| \href{http://www.alcyone.com/software/empy}{EmPy documentation} for more details | ||
| on its templating features and how to use them. |
There was a problem hiding this comment.
It might be worth noting here that empy is not bundled with Cylc.
|
@hjoliver, thanks for debugging this! Looks like |
@sadielbartholomew - as the architect of the new improved |
lib/parsec/empysupport.py
Outdated
| xtempl = StringIO() | ||
| interpreter = em.Interpreter(output=em.UncloseableFile(xtempl)) | ||
| interpreter.file(ftempl, '<template>', template_vars) | ||
| except Exception as exc: |
There was a problem hiding this comment.
Do lines 43-45 need to be inside this try block? If they raise an exception, it is presumably not due to an EmPy error.
There was a problem hiding this comment.
You're quite right! In fact line 46 should be outside the try block too, since error handling code uses interpreter object. Fixed now.
|
@hjoliver, yes it wouldn't take long to incorporate command-line arguments to only check for and output results for alternative software dependencies. While I do this I can add in options to ignore certain functionality needs for the current output e.g. requirements for one or either of the User Guide formats, as I am sure they will not be of concern to many users who might just view the User Guide on the web. I will add this to my To Do list. |
| It is a translation of \lstinline=jinja2.cities= example from | ||
| Section~\ref{Jinja2} and can be directly compared against it. | ||
|
|
||
| \lstset{language=suiterc} |
There was a problem hiding this comment.
I can't reply to your review summary, but let me just register a downvote at the use of the phrase 'lesser tools'.
There was a problem hiding this comment.
@oliver-sanders - what's your "eh?" refer to? The phrasing of "can be directly compared against it."? (should be "compared with it" I guess).
There was a problem hiding this comment.
Sorry, it was the lstset command.
There was a problem hiding this comment.
What's wrong with the lstset command?
There was a problem hiding this comment.
Nothing, I was reading it wrong.
|
@oliver-sanders and @sadielbartholomew - yes I suppose we should be more inclusive of those poor folks who choose to encumber themselves with "the lesser tools" 😁 It's fine with me though if we leave editor syntax file updates for other PRs, possibly by aficionados of said tools (since proper testing involves knowing how to use the editor). |
|
OK, I might create an issue to record this. [edit] #2752 |
|
The corresponding Rose issue metomi/rose#2217 is labelled with |
|
Agreed, going to next-release...and merging... |
This PR adds support for EmPy templating as an alternative to Jinja2. While it might seem of little benefit to add another templating system to Cylc, especially in light of future plans for proper APIs, I think that EmPy would be a much better fit for Cylc than Jinja2 and the added complexity to the Cylc codebase is very small.
Motivation:
Jinja2 project has a very rigid view of how templating should work and how it should be used. It follows the mantra of strict separation between presentation and logic (data) layers. While this might make sense for something like web pages, where there truly is a presentation layer, I would argue that Cylc configuration files do not fall into this category at all. In fact, templating of Cylc suites is used precisely to provide some additional logic during suite construction. This often starts simple, with a few
forloops and list variables to express some more repetitive parts of the suite, but over time grows with more and more state being held and passed around in various ad-hoc variables, lists and dictionaries. One might later attempt some modularization by using include files, macros and eventually custom filters, but it feels like Jinja2 is actively resisting, rather than helping, these efforts. Before long one ends up with substantial logic being trapped within Jinja2 templates, with no easy way to refactor it or migrate to something more modular.In contrast, EmPy is based around embedding straight Python code within templates and doesn't enforce any particular templating philosophy. As a result, it allows more uses cases than Jinja2 and integrates better with Python ecosystem. In simple use it doesn't looks very dissimilar to Jinja2 and offers the same basic control structures (for loops, if statements, macros - see the
examples/empy/cities/suite.rcfile in this PR for an example). The differences will start to show up, however, once the complexity of the template grows. One might implement some more complex logic as functions instead of macros, which in time might be refactored into one or more modules. This modularization can be accomplished much easier than in Jinja2, since the code being written is pure Python and there is no barrier to integrate it with the rest of Python ecosystem.While EmPy looks somewhat less polished and lacks some of the Jinja2 advanced features, like template inheritance (which is of little use for Cylc, however), it is more generic and not designed for any specific type of templating, like web pages. The project seems mature, but it's still actively maintained (there've been a bug fix release last year and the one before that was 4 years ago, so it's not a rapidly changing system). The license is LGPL, so compatible with Cylc.
TODO:
[empy:suite.rc]support in Rose (Empy support metomi/rose#2217)