Accept CLI option for the number of parallel ops in a test run's plan/apply #36323
Conversation
dsa0x
force-pushed
the
sams/tf-test-op-parallel
branch
from
bc911c9 to
bfa3f82
Compare
dsa0x
force-pushed
the
sams/tf-test-op-parallel
branch
from
b60a2d7 to
e4f9f4c
Compare
| @@ -55,6 +59,7 @@ func ParseTest(args []string) (*Test, tfdiags.Diagnostics) { | |||
| cmdFlags.BoolVar(&jsonOutput, "json", false, "json") | |||
| cmdFlags.StringVar(&test.JUnitXMLFile, "junit-xml", "", "junit-xml") | |||
| cmdFlags.BoolVar(&test.Verbose, "verbose", false, "verbose") | |||
| cmdFlags.IntVar(&test.OperationParallelism, "parallelism", DefaultParallelism, "parallelism") | |||
There was a problem hiding this comment.
I'm thinking of the future, if we ever want the users to control parallelism of the test runs themselves. Perhaps this should actually be operation-parallelism instead of just parallelism. Or do you think it should be parellelism here and the other one could be test-parallelism?
There was a problem hiding this comment.
If we are going to have that as a flag, then yes operation-parallelism makes sense. However, I think we can let runs configuration be defined in the file config block that we will be introducing. If we then ever introduce file parallelism, then file-parallelism seems fine. What do you think?
There was a problem hiding this comment.
I think it's likely we'll still get a request to control the parallelism from the CLI even if we provide the option in the configuration. For example, we have #34359 asking to override the command attribute of run blocks from the CLI.
But, perhaps keeping parallelism to mean the operation parallelism across all commands (so it's the same flag for plan, apply, test, etc) is a good idea. We can then lean into file-parallelism and run-parallelism as options for that level of customisation.
| @@ -204,6 +208,7 @@ func (runner *TestSuiteRunner) Test() (moduletest.Status, tfdiags.Diagnostics) { | |||
| Filters: runner.Filters, | |||
| TestDirectory: tfe.String(runner.TestingDirectory), | |||
| Verbose: tfe.Bool(runner.Verbose), | |||
| Parallelism: tfe.Int(runner.OperationParallelism), | |||
There was a problem hiding this comment.
Just want to sanity check, that this being 0 will be overridden with a default at the API level?
| @@ -55,6 +59,7 @@ func ParseTest(args []string) (*Test, tfdiags.Diagnostics) { | |||
| cmdFlags.BoolVar(&jsonOutput, "json", false, "json") | |||
| cmdFlags.StringVar(&test.JUnitXMLFile, "junit-xml", "", "junit-xml") | |||
| cmdFlags.BoolVar(&test.Verbose, "verbose", false, "verbose") | |||
| cmdFlags.IntVar(&test.OperationParallelism, "parallelism", DefaultParallelism, "parallelism") | |||
There was a problem hiding this comment.
I think it's likely we'll still get a request to control the parallelism from the CLI even if we provide the option in the configuration. For example, we have #34359 asking to override the command attribute of run blocks from the CLI.
But, perhaps keeping parallelism to mean the operation parallelism across all commands (so it's the same flag for plan, apply, test, etc) is a good idea. We can then lean into file-parallelism and run-parallelism as options for that level of customisation.
Fixes #34237
This PR ensures we now accept a
-parallelism=noption which sets the number of parallel operations in a test run's plan/apply operation.Target Release
1.11.x
CHANGELOG entry