Repository navigation
Terraform: upgrade python-hcl2 to 8.1.4 - #23740
Conversation
a33468d to
826b6f4
Compare
tdyas
left a comment
There was a problem hiding this comment.
Thanks for fixing!
Would it make sense to just require the newer version and thus avoid the need to check for SerializationOptions?
Yeah, probably. I was a little worried about breaking anyone that had this pinned, but with other similar breaking changes it looks like the pattern is to just leave a note in the release notes/upgrade instructions. I'll rework this a bit to pin to the current version or better. |
We could also deprecate for one version, and then remove in the subsequent version. Maybe add a deprecation warning using |
fd0f6cf to
082104a
Compare
I reworked things to use the deprecation code/messaging. I'm happy with this current state and I think this is good to go now. |
|
|
||
| # Printed to stderr when the installed python-hcl2 is older than 8.0, so that Pants can warn that | ||
| # support for it is deprecated. | ||
| LEGACY_HCL2_MARKER = "__pants_legacy_python_hcl2__" |
There was a problem hiding this comment.
This can be a bytes b"" constant and then the various LEGACY_HCL2_MARKER.encode() calls can be avoided.
There was a problem hiding this comment.
Just the change and the PR otherwise looks good to go.
python-hcl2 8.x keeps the quotes around string values unless `SerializationOptions(strip_string_quotes=True)` is passed, so the parser script now passes it. Without it, module `source` values never matched a local path and no module dependencies were inferred. Raise the default requirement to `python-hcl2>=8.1.4,<9` and upgrade the bundled lockfile from 4.3.0 to 8.1.4, the first 8.x release that parses `for` as an attribute name (pantsbuild#21845), `in` as a block name (pantsbuild#22401), and conditional expressions split over multiple lines. A resolve set with `[terraform-hcl2-parser].install_from_resolve` can still provide a python-hcl2 release older than 8.0, but that is deprecated: the parser falls back to the old API, which has no `SerializationOptions`, and Pants logs a deprecation warning. Support is scheduled for removal in 2.35.0.dev0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
082104a to
028ef85
Compare
Upgrade the
python-hcl2version bundled with theterraform-hcl2-parsersubsystem from 4.3.0 to 8.1.4.4.3.0 rejects some valid Terraform, and one unparseable file fails dependency inference for its whole
terraform_module. 8.1.4 parses all of these:forused as an attribute name (lark.exceptions.UnexpectedTokenwhen creating alert rules via Grafana's Terraform Provider #21845)inused as a block name (lark.exceptions.UnexpectedTokenWhen creating Snowflake-Terraform data source outpus using database params #22401)python-hcl2 8.x keeps the quotes around string values by default, so
source = "./foo"comes back as"\"./foo\"". With the currenthcl2_parser.py, that means nosourceever looks like a local path, and Pants silently infers no module dependencies at all. The parser now passesSerializationOptions(strip_string_quotes=True).default_requirementsis nowpython-hcl2>=8.1.4,<9, andhcl2.lockwas regenerated withgenerate_builtin_lockfiles.py, which also addsregex, a new dependency of 8.x.As suggested in review, older releases are deprecated for one version instead of being dropped right away. A resolve set with
[terraform-hcl2-parser].install_from_resolvecan still provide python-hcl2 older than 8.0. Those releases have noSerializationOptionsand strip the quotes themselves, so the parser falls back to plainhcl2.loads(content)and prints a marker to stderr. When dependency inference sees the marker, it logs a deprecation warning, scheduled for removal in 2.35.0.dev0, that tells users to upgrade that resolve to 8.1.4 or newer.docs/notes/2.34.x.mdcovers both the upgrade and the deprecation.Pants pinned 4.3.0 in #21361 because of parse regressions in 4.3.1+. To check for regressions, I ran the patched parser over a private codebase with ~900
.tffiles. 8.1.4 inferred the same 106 module dependencies as 4.3.0 and parsed every file that belongs to aterraform_module.Testing:
test_hcl_parser_handles_keywords_and_multiline_conditionalsparses the three constructs above with the bundled 8.1.4.test_hcl_parser_wrapper_runspasses on Python 3.10 through 3.14. It and the new test also check that 8.1.4 isn't reported as a deprecated release.hcl2_parser_test.py,test_main_strips_string_quotesandtest_main_falls_back_for_legacy_hcl2use stand-inhcl2modules to check that the parser asks 8.x to strip quotes, and that it falls back and prints the marker for releases withoutSerializationOptions.test_warn_if_legacy_hcl2checks that the marker produces the deprecation warning.pants fmt lint checkpasses on the changed files.This code was generated with Claude code, however I am using this on my internal systems and so far it is working well in my pipelines.