https://pantsbuild.org/ logo
Join Slack
Powered by
<@U0N6C2Q9F> Any insight on this terraform commit?...
# development
w
@fast-nail-55400 Any insight on this terraform commit? https://github.com/pantsbuild/pants/commit/3cdd8987b9f1eaf4340134f200120bcb69efca70 I pyupgraded the repo syntax to 3.11, but it's failing on a terraform test that runs on python3.8 (https://github.com/pantsbuild/pants/actions/runs/13115852226/job/36590017998)
s
Looks like it explicitly sets python version
Copy code
major_minor_interpreter = '3.8'
Have you tried switching it to 3.11?
w
Yeah, it's a parametrized test using 
all_major_minor_python_versions
- but the underlying question is more like... why? Do we need this? etc. Works fine on my machine using 3.11 - since the error has to do with not understanding 
set
or something similar
šŸ¤” 1
c
The Python version param stuff feels inconsistent to me. I think the question is, for 
install_from_resolve
subsystems where one can change the interpreter constraints, do we support: ā€¢ All Python versions that Pants itself supports for python_sources. Which would currently be >=3.8 after we dropped 3.7 following pip's lead. ā€¢ Python version >= the version of Pants itself. Although now that I've typed this, in this narrow case, wouldn't we just need to exclude that file from the upgrade since it is a special case? Like in 
src/python/pants/backend/python/util_rules/scripts/BUILD
w
Sure - but, is it just a blatant narrow case? Or are there other hidden narrow cases that this upgrade pastes over
I don't use the terraform pants hooks, so this error is a bit confusing in the first place - I think I need to look at this HCL parser and make sure this is a "real" error and not just a test error
Okay, yeah, it's kinda weird - we use that file in pants and copy that out, to later be run on. "# Note: This file is used as a pex entry point in the execution sandbox." I'd question whether that file should really live where it does, rather than being a template file or having a different extension It's more like a build-script we have put in build-support or a few other places before. It's not really part of the pants machinery per se šŸ¤· I'll just skip pyupgrade and move on to the next error
c
On closer inspection, I think this file is a special case (like the 2.7 one at 
src/python/pants/backend/python/util_rules/scripts/BUILD
)
w
Yeah, I ignored it from pyupgrade - because yeah, it's not part of Pants really. It's something that gets run that happens to be placed inside of the pants codebase
f
Looking through above.
For some history of why Terraform dependency inference is structured this way, it is to avoid needing the 
hcl2
dependency in the main Pants resolve, which I recall had been a review comment.
And yes, please exclude the file in question from any Pants code base upgrades since technically that file is more like a script resource run indirectly, rather than an actual part of the main Pants code base.
(Similar idea to the JVM and Go code we have in the Pants repository. This just happens to be Python.)
w
šŸ‘ Thanks for the feedback. I'm a few mypy hell-holes away from the syntax update working
Interestingly, our mypy settings seem to treat Type and type differently... That's something
2 Views
Open in Slack
PreviousNext
Powered by