https://pantsbuild.org/ logo
Powered by
b

bitter-ability-32190

09/22/2022, 5:44 PM
@fast-nail-55400 for 
terraform fmt
is it OK if we're formatting a subset of files in a directory? E.g. if a directory has 
[a, b, c]
and we fmt 
[a, b]
in one process and 
[c]
in another?
f

fast-nail-55400

09/22/2022, 5:49 PM
[side note: Better to ask on the PR so the knowledge is in the place where it is needed.] Not okay with Terraform since the 
terraform fmt
invocation is only passed the directory and not the files therein.
The Terraform backend is similar to the Go backend in that the entire Terraform module is what Pants acts on.
b

bitter-ability-32190

09/22/2022, 5:49 PM
Right, but the snapshot will contain the subset of files in that dir
(this is for an upcoming change, not PR 16966)
f

fast-nail-55400

09/22/2022, 5:50 PM
And within a single Terraform module (which is synonymous with directory for Terraform) Terraform will attempt to process all 
.tf
files in that directory.
So for Terraform, Pants partitions should not try and split files in the same directory into different partitions.
b

bitter-ability-32190

09/22/2022, 5:52 PM
f

fast-nail-55400

09/22/2022, 5:52 PM
note: this is why the Terraform fmt support partitions files by directory
because it then uses the list of directories as the args to Terraform, not the files
b

bitter-ability-32190

09/22/2022, 5:53 PM
Just to be crystal clear. Given 
dir/A.tf dir/B.tf dir/C.tf
It's incorrect for Pants to produce 2 processes: 1. A process with a snapshot of just 
dir/A.tf
, process args are 
dir
2. A process with a snapshot of 
dir/B.tf
and 
dir/C.tf
process args are 
dir
f

fast-nail-55400

09/22/2022, 5:53 PM
note that 
go vet
may have a similar requirement if it needs to see the whole 
go_package
correct it is incorrect to do that especially for semantic linting like 
terraform validate
. All three files constitute the Terraform module and they can have circular references to each other without need to do import symbols or anything.
so partitioning that way is in fact semantically incorrect since it breaks apart the Terraform module
b

bitter-ability-32190

09/22/2022, 5:55 PM
Looks like I have some tweaking to do 😛
f

fast-nail-55400

09/22/2022, 5:57 PM
(this is for an upcoming change, not PR 16966)
ack. I misunderstood then.
b

bitter-ability-32190

09/22/2022, 5:57 PM
So just to make sure I understand how the code works today, It's OK for 
lint
to "batch" the inputs because it does so by target and 
terraform_module
is a single target representing multiple files?
f

fast-nail-55400

09/22/2022, 5:58 PM
correct. That is why there are no file-level targets for Terraform modules (nor for Go packages for that matter).
side note: You could technically get away with batching by file for 
terraform fmt
since all it does is format files, but that would not likely work for 
terraform validate
which does semantic checks. And I would classify "trying get away with it" in this case to be a hack around good abstraction on how we modeled Terraform.
b

bitter-ability-32190

09/22/2022, 8:13 PM
You could technically get away with batching by file for 
terraform fmt
since all it does is format files,
In order to make progress, I think I'm going to move forward doing this, and file a ticket to collaborate on a better solution to batching/partitioning.
f

fast-nail-55400

09/25/2022, 6:01 PM
My concern with that approach is that I fear a departure from how 
terraform
models modules could end up in place for an extended period of time due to lack of developer time to make the fix.
b

bitter-ability-32190

09/25/2022, 6:11 PM
We have some ideas on how to fix it, to some degree. But I 'd prefer that be in a separate, dedicated PR
Also FWIW it's rare it'll occur, we think. Either there has to be more files than a single batch (somewhere in the hundreds) or a file formatter tries to format a strict subset of files in the dir.
Powered by