Skip to content

Commit

Permalink
Rename SubPartition to Batch in partitioning types. (pantsbuild#1…
Browse files Browse the repository at this point in the history
…7263)

We've been mixing terminology a bit in subsystems that partition inputs. After this commit, we use terms (more) consistently to mean:

  1. "Partition": a set of compatible inputs that _can_ be processed together, but might not be depending on other settings
  2. "Batch": a set of inputs that are actually processed together
  • Loading branch information
danxmoran authored Oct 19, 2022
1 parent be00ed5 commit 08410e5
Show file tree
Hide file tree
Showing 70 changed files with 263 additions and 221 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ In order to support targetless formatters, `fmt` needs to know which _files_ you
Therefore the plugin API for `fmt` has forked into 2 rules:

1. A rule taking `<RequestType>.PartitionRequest` and returning a `Partitions` object. This is sometimes referred to as the "partitioner" rule.
2. A rule taking `<RequestType>.SubPartition` and returning a `FmtResult`. This is sometimes referred to as the "runner" rule.
2. A rule taking `<RequestType>.Batch` and returning a `FmtResult`. This is sometimes referred to as the "runner" rule.

This way `fmt` can serialize tool runs that operate on the same file(s) while parallelizing tool runs
that don't overlap.
Expand All @@ -50,7 +50,7 @@ The partitioner rule gives you an opportunity to perform expensive `Get`s once f
to partition the inputs based on metadata to simplify your runner, and to have a place for easily
skipping your tool if requested.

The runner rule will mostly remain unchanged, aside from the request type (`<RequestType>.SubPartition`),
The runner rule will mostly remain unchanged, aside from the request type (`<RequestType>.Batch`),
which now has a `.files` property.

If you don't require any `Get`s or metadata for your tool in your partitioner rule,
Expand All @@ -65,7 +65,7 @@ Lint:
Lint plugins are almost identical to format plugins, except in 2 ways:

1. Your partitioner rule still returns a `Partitions` object, but the element type can be anything.
2. `<RequestType>.SubPartition` has a `.elements` field instead of `.files`.
2. `<RequestType>.Batch` has a `.elements` field instead of `.files`.

-----

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ class ShfmtRequest(FmtTargetsRequest):

# 3. Create `fmt` rules

You will need a rule for `fmt` which takes the `FmtTargetsRequest.SubPartition` from step 3 (e.g. `ShfmtRequest`) as a parameter and returns a `FmtResult`.
You will need a rule for `fmt` which takes the `FmtTargetsRequest.Batch` from step 3 (e.g. `ShfmtRequest`) as a parameter and returns a `FmtResult`.

```python
@rule(desc="Format with shfmt", level=LogLevel.DEBUG)
async def shfmt_fmt(request: ShfmtRequest.SubPartition, shfmt: Shfmt, platform: Platform) -> FmtResult:
async def shfmt_fmt(request: ShfmtRequest.Batch, shfmt: Shfmt, platform: Platform) -> FmtResult:
download_shfmt_get = Get(
DownloadedExternalTool,
ExternalToolRequest,
Expand Down Expand Up @@ -152,7 +152,7 @@ async def shfmt_fmt(request: ShfmtRequest.SubPartition, shfmt: Shfmt, platform:
return await FmtResult.create(request, result, output_snapshot)
```

The `FmtRequest.SubPartition` has `.snapshot`, which stores the list of files and the `Digest` for each source file.
The `FmtRequest.Batch` has `.snapshot`, which stores the list of files and the `Digest` for each source file.

If you used `ExternalTool` in step 1, you will use `Get(DownloadedExternalTool, ExternalToolRequest)` to ensure that the tool is fetched.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ class ShellcheckRequest(LintTargetsRequest):

# 3. Create a rule for your linter logic

Your rule should take as a parameter `ShellcheckRequest.SubPartition` and the
`Subsystem` (or `ExternalTool`) from step 1 (a `SubPartition` is an object containing a subset of all
Your rule should take as a parameter `ShellcheckRequest.Batch` and the
`Subsystem` (or `ExternalTool`) from step 1 (a `Batch` is an object containing a subset of all
the matched field sets for your tool). It should return a `LintResult`:

```python
Expand All @@ -117,7 +117,7 @@ async def run_shellcheck(
return LintResult.create(...)
```

The `ShellcheckRequest.SubPartition` instance has a property called `.elements`, which in this case,
The `ShellcheckRequest.Batch` instance has a property called `.elements`, which in this case,
stores a collection of the `FieldSet`s defined in step 2. Each `FieldSet` corresponds to a single target.
Pants will have already validated that there is at least one valid `FieldSet`.

Expand Down Expand Up @@ -158,7 +158,7 @@ from pants.util.strutil import pluralize

@rule
async def run_shellcheck(
request: ShellcheckRequest.SubPartition, shellcheck: Shellcheck, platform: Platform
request: ShellcheckRequest.Batch, shellcheck: Shellcheck, platform: Platform
) -> LintResult:
download_shellcheck_request = Get(
DownloadedExternalTool,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def rule_runner() -> RuleRunner:
*black_fmt_rules(),
*black_subsystem_rules(),
*config_files.rules(),
QueryRule(FmtResult, (BlackRequest.SubPartition,)),
QueryRule(FmtResult, (BlackRequest.Batch,)),
],
target_types=[PythonSourcesGeneratorTarget],
)
Expand Down Expand Up @@ -61,7 +61,7 @@ def run_black(rule_runner: RuleRunner, *, extra_args: list[str] | None = None) -
fmt_result = rule_runner.request(
FmtResult,
[
BlackRequest.SubPartition("", snapshot.files, key=None, snapshot=snapshot),
BlackRequest.Batch("", snapshot.files, partition_key=None, snapshot=snapshot),
],
)
return fmt_result
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/build_files/fmt/black/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class BlackRequest(FmtBuildFilesRequest):


@rule(desc="Format with Black", level=LogLevel.DEBUG)
async def black_fmt(request: BlackRequest.SubPartition, black: Black) -> FmtResult:
async def black_fmt(request: BlackRequest.Batch, black: Black) -> FmtResult:
black_ics = await Black._find_python_interpreter_constraints_from_lockfile(black)
return await _run_black(request, black, black_ics)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class BuildifierRequest(FmtBuildFilesRequest):

@rule(desc="Format with Buildifier", level=LogLevel.DEBUG)
async def buildfier_fmt(
request: BuildifierRequest.SubPartition, buildifier: Buildifier, platform: Platform
request: BuildifierRequest.Batch, buildifier: Buildifier, platform: Platform
) -> FmtResult:
buildifier_tool = await Get(
DownloadedExternalTool, ExternalToolRequest, buildifier.get_request(platform)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def rule_runner() -> RuleRunner:
*buildifier_rules(),
*external_tool.rules(),
*target_types_rules(),
QueryRule(FmtResult, [BuildifierRequest.SubPartition]),
QueryRule(FmtResult, [BuildifierRequest.Batch]),
],
# NB: Objects are easier to test with
objects={"materials": Materials},
Expand Down Expand Up @@ -62,7 +62,7 @@ def run_buildifier(rule_runner: RuleRunner) -> FmtResult:
fmt_result = rule_runner.request(
FmtResult,
[
BuildifierRequest.SubPartition("", snapshot.files, key=None, snapshot=snapshot),
BuildifierRequest.Batch("", snapshot.files, partition_key=None, snapshot=snapshot),
],
)
return fmt_result
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def rule_runner() -> RuleRunner:
*yapf_fmt_rules(),
*yapf_subsystem_rules(),
*config_files.rules(),
QueryRule(FmtResult, (YapfRequest.SubPartition,)),
QueryRule(FmtResult, (YapfRequest.Batch,)),
],
target_types=[PythonSourcesGeneratorTarget],
)
Expand All @@ -42,7 +42,7 @@ def run_yapf(rule_runner: RuleRunner, *, extra_args: list[str] | None = None) ->
fmt_result = rule_runner.request(
FmtResult,
[
YapfRequest.SubPartition("", snapshot.files, key=None, snapshot=snapshot),
YapfRequest.Batch("", snapshot.files, partition_key=None, snapshot=snapshot),
],
)
return fmt_result
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/build_files/fmt/yapf/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class YapfRequest(FmtBuildFilesRequest):


@rule(desc="Format with Yapf", level=LogLevel.DEBUG)
async def yapf_fmt(request: YapfRequest.SubPartition, yapf: Yapf) -> FmtResult:
async def yapf_fmt(request: YapfRequest.Batch, yapf: Yapf) -> FmtResult:
yapf_ics = await Yapf._find_python_interpreter_constraints_from_lockfile(yapf)
return await _run_yapf(request, yapf, yapf_ics)

Expand Down
4 changes: 1 addition & 3 deletions src/python/pants/backend/cc/lint/clangformat/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ class ClangFormatRequest(FmtTargetsRequest):


@rule(level=LogLevel.DEBUG)
async def clangformat_fmt(
request: ClangFormatRequest.SubPartition, clangformat: ClangFormat
) -> FmtResult:
async def clangformat_fmt(request: ClangFormatRequest.Batch, clangformat: ClangFormat) -> FmtResult:

# Look for any/all of the clang-format configuration files (recurse sub-dirs)
config_files_get = Get(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def rule_runner() -> RuleRunner:
*source_files.rules(),
*config_files.rules(),
*target_types_rules.rules(),
QueryRule(FmtResult, (ClangFormatRequest.SubPartition,)),
QueryRule(FmtResult, (ClangFormatRequest.Batch,)),
QueryRule(SourceFiles, (SourceFilesRequest,)),
],
target_types=[CCSourcesGeneratorTarget],
Expand Down Expand Up @@ -98,8 +98,11 @@ def run_clangformat(
fmt_result = rule_runner.request(
FmtResult,
[
ClangFormatRequest.SubPartition(
"", input_sources.snapshot.files, key=None, snapshot=input_sources.snapshot
ClangFormatRequest.Batch(
"",
input_sources.snapshot.files,
partition_key=None,
snapshot=input_sources.snapshot,
),
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ async def partition_buf(

@rule(desc="Format with buf format", level=LogLevel.DEBUG)
async def run_buf_format(
request: BufFormatRequest.SubPartition, buf: BufSubsystem, platform: Platform
request: BufFormatRequest.Batch, buf: BufSubsystem, platform: Platform
) -> FmtResult:
diff_binary = await Get(DiffBinary, DiffBinaryRequest())
download_buf_get = Get(DownloadedExternalTool, ExternalToolRequest, buf.get_request(platform))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def rule_runner() -> RuleRunner:
*external_tool.rules(),
*source_files.rules(),
*target_types_rules(),
QueryRule(FmtResult, [BufFormatRequest.SubPartition]),
QueryRule(FmtResult, [BufFormatRequest.Batch]),
QueryRule(SourceFiles, [SourceFilesRequest]),
],
target_types=[ProtobufSourcesGeneratorTarget],
Expand Down Expand Up @@ -62,8 +62,11 @@ def run_buf(
fmt_result = rule_runner.request(
FmtResult,
[
BufFormatRequest.SubPartition(
"", input_sources.snapshot.files, key=None, snapshot=input_sources.snapshot
BufFormatRequest.Batch(
"",
input_sources.snapshot.files,
partition_key=None,
snapshot=input_sources.snapshot,
),
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ async def partition_buf(

@rule(desc="Lint with buf lint", level=LogLevel.DEBUG)
async def run_buf(
request: BufLintRequest.SubPartition[Any, BufFieldSet], buf: BufSubsystem, platform: Platform
request: BufLintRequest.Batch[Any, BufFieldSet], buf: BufSubsystem, platform: Platform
) -> LintResult:
transitive_targets = await Get(
TransitiveTargets,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def rule_runner() -> RuleRunner:
*stripped_source_files.rules(),
*target_types_rules(),
QueryRule(Partitions, [BufLintRequest.PartitionRequest]),
QueryRule(LintResult, [BufLintRequest.SubPartition]),
QueryRule(LintResult, [BufLintRequest.Batch]),
],
target_types=[ProtobufSourcesGeneratorTarget],
)
Expand Down Expand Up @@ -60,10 +60,10 @@ def run_buf(
[BufLintRequest.PartitionRequest(tuple(BufFieldSet.create(tgt) for tgt in targets))],
)
results = []
for key, subpartition in partition.items():
for key, batch in partition.items():
result = rule_runner.request(
LintResult,
[BufLintRequest.SubPartition("", subpartition, key)],
[BufLintRequest.Batch("", batch, key)],
)
results.append(result)
return tuple(results)
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/docker/lint/hadolint/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def generate_argv(

@rule(desc="Lint with Hadolint", level=LogLevel.DEBUG)
async def run_hadolint(
request: HadolintRequest.SubPartition[Any, HadolintFieldSet],
request: HadolintRequest.Batch[Any, HadolintFieldSet],
hadolint: Hadolint,
platform: Platform,
) -> LintResult:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def rule_runner() -> RuleRunner:
package.find_all_packageable_targets,
*source_files.rules(),
QueryRule(Partitions, [HadolintRequest.PartitionRequest]),
QueryRule(LintResult, [HadolintRequest.SubPartition]),
QueryRule(LintResult, [HadolintRequest.Batch]),
],
target_types=[DockerImageTarget],
)
Expand All @@ -53,10 +53,10 @@ def run_hadolint(
[HadolintRequest.PartitionRequest(tuple(HadolintFieldSet.create(tgt) for tgt in targets))],
)
results = []
for key, subpartition in partition.items():
for key, batch in partition.items():
result = rule_runner.request(
LintResult,
[HadolintRequest.SubPartition("", subpartition, key)],
[HadolintRequest.Batch("", batch, key)],
)
results.append(result)
return tuple(results)
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/go/lint/gofmt/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class GofmtRequest(FmtTargetsRequest):


@rule(desc="Format with gofmt")
async def gofmt_fmt(request: GofmtRequest.SubPartition, goroot: GoRoot) -> FmtResult:
async def gofmt_fmt(request: GofmtRequest.Batch, goroot: GoRoot) -> FmtResult:
argv = (
os.path.join(goroot.path, "bin/gofmt"),
"-w",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def rule_runner() -> RuleRunner:
*build_pkg.rules(),
*link.rules(),
*assembly.rules(),
QueryRule(FmtResult, (GofmtRequest.SubPartition,)),
QueryRule(FmtResult, (GofmtRequest.Batch,)),
QueryRule(SourceFiles, (SourceFilesRequest,)),
],
)
Expand Down Expand Up @@ -120,8 +120,11 @@ def run_gofmt(
fmt_result = rule_runner.request(
FmtResult,
[
GofmtRequest.SubPartition(
"", input_sources.snapshot.files, key=None, snapshot=input_sources.snapshot
GofmtRequest.Batch(
"",
input_sources.snapshot.files,
partition_key=None,
snapshot=input_sources.snapshot,
),
],
)
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/go/lint/golangci_lint/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class GolangciLintRequest(LintTargetsRequest):

@rule(desc="Lint with golangci-lint", level=LogLevel.DEBUG)
async def run_golangci_lint(
request: GolangciLintRequest.SubPartition[Any, GolangciLintFieldSet],
request: GolangciLintRequest.Batch[Any, GolangciLintFieldSet],
golangci_lint: GolangciLint,
goroot: GoRoot,
bash: BashBinary,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def rule_runner() -> RuleRunner:
*target_type_rules.rules(),
*third_party_pkg.rules(),
QueryRule(Partitions, [GolangciLintRequest.PartitionRequest]),
QueryRule(LintResult, [GolangciLintRequest.SubPartition]),
QueryRule(LintResult, [GolangciLintRequest.Batch]),
SubsystemRule(GolangciLint),
],
)
Expand Down Expand Up @@ -108,10 +108,8 @@ def run_golangci_lint(
],
)
results = []
for key, subpartition in partition.items():
result = rule_runner.request(
LintResult, [GolangciLintRequest.SubPartition("", subpartition, key)]
)
for key, batch in partition.items():
result = rule_runner.request(LintResult, [GolangciLintRequest.Batch("", batch, key)])
results.append(result)
return tuple(results)

Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/go/lint/vet/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class GoVetRequest(LintTargetsRequest):


@rule(level=LogLevel.DEBUG)
async def run_go_vet(request: GoVetRequest.SubPartition[Any, GoVetFieldSet]) -> LintResult:
async def run_go_vet(request: GoVetRequest.Batch[Any, GoVetFieldSet]) -> LintResult:
source_files = await Get(
SourceFiles,
SourceFilesRequest(field_set.sources for field_set in request.elements),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def rule_runner() -> RuleRunner:
*build_pkg.rules(),
*assembly.rules(),
QueryRule(Partitions, [GoVetRequest.PartitionRequest]),
QueryRule(LintResult, [GoVetRequest.SubPartition]),
QueryRule(LintResult, [GoVetRequest.Batch]),
SubsystemRule(GoVetSubsystem),
],
)
Expand Down Expand Up @@ -100,10 +100,10 @@ def run_go_vet(
[GoVetRequest.PartitionRequest(tuple(GoVetFieldSet.create(tgt) for tgt in targets))],
)
results = []
for key, subpartition in partition.items():
for key, batch in partition.items():
result = rule_runner.request(
LintResult,
[GoVetRequest.SubPartition("", subpartition, key)],
[GoVetRequest.Batch("", batch, key)],
)
results.append(result)
return tuple(results)
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/helm/goals/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ async def partition_helm_lint(

@rule(desc="Lint Helm charts", level=LogLevel.DEBUG)
async def run_helm_lint(
request: HelmLintRequest.SubPartition[HelmChart, HelmLintFieldSet],
request: HelmLintRequest.Batch[HelmChart, HelmLintFieldSet],
helm_subsystem: HelmSubsystem,
) -> LintResult:
assert len(request.elements) == 1
field_set = request.elements[0]
chart = request.key
chart = request.partition_key

argv = ["lint", chart.name]

Expand Down
Loading

0 comments on commit 08410e5

Please sign in to comment.