jinja2: move to version 3

[oliver-sanders]

Jinja2 has made it out of the 2.x series into the 3.x series (but is still called Jinja2).

Historically we have had to pin cylc-flow to exact versions of Jinja2 due to a series of highly disruptive breaking changes made during the 2.x series in order to give our users a fighting chance.

We should consider moving to version 3, however, do so cautiously. Suggest rendering a particularly complex workflow configuration with Jinja2 2.11 then with 3.? and comparing the output for piece of mind.

Questions:

• When to update, 8.0.0, 8.1.0, 8.x? Dependent on the presence or absence of breaking changes?
• Should we continue to pin to minor releases?

Pull requests welcome!

[hjoliver]

(but is still called Jinja2).

Haha 🤣

[hjoliver]

• When to update ?

8.0, in case there are breaking changes (which we know about or not) from the Jinja2 upgrade.

[dpmatthews]

Discussion: we will test against example workflows with advanced Jinja2 usage to check for any compatibility issues

[hjoliver]

If it causes problems, we may need to resort to parallel installations of Cylc 8 for a while, one with old-Jinja2 for use with Cylc 7 back-compat mode. (Worst case scenario!).

[oliver-sanders]

Have begun testing our workflows with Jinja2 v3, unfortunately one issue has show up very quickly, like Python Jinja2 now considers an integer with a leading zero to be a syntax error:

# python 2
foo = 4  # fine
foo = 04  # octal

# jinja2 v2
foo = 4  # fine
foo = 04  # fine, integer presumably?

# python 3
foo = 4  # fine
foo = 04  # error
foo = o04  # octal

# jinja2 v3
foo = 4  # fine
foo = 04  # error
foo = o04  # octal presumably

This issue is probably very widespread and has impacts to both suite.rc+Jinja2 & rose-suite.conf[jinja2:suite.rc].

[oliver-sanders]

Besides the occasional (and unavoidable Python 2->3 error) I'm also finding substantial diffs in the graph sections of complex workflows. It looks like subtle line order changes but would have to confirm that, strange that it's only in Graph sections (I'm using cylc view --process so graph section merging should not impact results).

[hjoliver]

Damn it 😬

[oliver-sanders]

The good news is that the diffs I was seeing were the result of a random element in the workflow definition and not the Jinja2 upgrade.

The bad news is that the leading-zero issue is widespread and quite annoying. We've been here before and quickly realised this was way too disruptive so put in logic to back-support this:

https://github.com/cylc/cylc-rose/blob/907a68065f39e17d2ab7f5173297743d91469061/cylc/rose/jinja2_parser.py#L82-L84

With the Jinja2 upgrade this test now fails and the problem returns:

082             # valid jinja2 variants
083             >>> parser.literal_eval('042')
UNEXPECTED EXCEPTION: TemplateSyntaxError("expected token 'end of print statement', got 'integer'")

Since we have already made the decision to back-support Jinja2 native syntax for template variables in the rose-suite.conf file, one option would be to fudge the template variable parsing in cylc-rose to strip the leading zero before it gets to the Jinja2 evaluation.

Zero-padded integers would still be invalid if used within the Cylc workflow files:

# flow.cylc
{% set FOO=01 %}  # ERROR

However, would continue to be supported in the rose-suite.conf file:

# rose-suite.conf
[template variables]
FOO=01  # fine (same for Rose 2 same for Rose 2019)

[oliver-sanders]

Update: have managed to work out a way to back support integers with leading zeros in Jinja2 for Cylc-Rose. This allows us to issue deprecation warnings to manage the transition.

[oliver-sanders]

Update: Solution to support integers with leading zeros here - cylc/cylc-rose#140

[MetRonnie]

Having tested jinja2 v3.0 (and cylc/cylc-rose#140) using

cylc view --jinja2 --stdout

against ~60,000 source suites at the MO, we have found only ~200 suites that fail in jinja2 v3.0 but not v2.x. All of these are due to leading zeros in integers. There are no suites where there is any difference in output between v3.0 and v2.x.

I think we should probably put a note in the 7-to-8 docs about zero-padded integers. Perhaps in https://cylc.github.io/cylc-doc/nightly/html/7-to-8/major-changes/python-2-3.html? Or we can add a Jinja2 page to the Major Changes dir, but there isn't that much to say, just that the error message you get from Jinja2 is not very clear and I think we ought to document that leading zeros are the culprit

Jinja2Error: expected token ',', got 'integer'

[oliver-sanders]

Thanks @MetRonnie!

We have a temporary solution to supporting zero-padded integers in the rose-suite.conf file where most of them are defined (via cylc-rose) - cylc/cylc-rose#140. The small remainder of cases where zero-padded integers are used in the suite.rc files can be fixed by hand.

With no other issues to report I think we are safe to move ahead with this at Cylc 8.0.0 without threatening Cylc7/8 interoperability or posing a major adoption barrier 🎉. Replacing the "investigation" label with small (update "setup.cfg" and "conda-environment.yml") and "doc" (see #4702 (comment)).

[oliver-sanders]

Note we must move to Jinja2 3.0.* not 3.* initially as Jinja2 deprecated a bunch of things in 3.0.0 (with warnings) and removed these interfaces at 3.1.0.

Moving to 3.1.0 would probably kill Cylc 7/8 interoperability for workflows which use Jinja2 globals/filters or import jinja2 via {% import so we will probably have to stick with 3.0.* until we are ready to cut off Cylc 7 interoperability.

Note we will want to advise users who wish to maintain Cylc 7 interoperability not to take action on Jinja2 warnings until then.