Skip to content

Commit

Permalink
Deprecate some Request types in favor of Get() with only one arg (p…
Browse files Browse the repository at this point in the history
…antsbuild#18655)

Closes pantsbuild#17342.

I found the stale APIs based on my memory and using this regex: `rg
'\bGet\(([A-Za-z0-9]+),\s*((?![A-Za-z0-9]*Sentinel\(\))[A-Za-z0-9]+)\(\)\)'
--pcre2` (thanks ChatGPT4 🙇)
  • Loading branch information
Eric-Arellano authored Apr 4, 2023
1 parent 9046bb7 commit 02e0ba2
Show file tree
Hide file tree
Showing 19 changed files with 239 additions and 153 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,35 @@ hidden: false
createdAt: "2020-10-12T16:19:01.543Z"
---

2.17
----

### Deprecated some `Request` types in favor of `Get` with only one arg

For several APIs like `pants.core.util_rules.system_binaries`, we had an eager and lazy version of the same API. You could do either of these two:

```python
from pants.core.util_rules.system_binaries import ZipBinary, ZipBinaryRequest
from pants.engine.rules import Get, rule

class MyOutput:
pass

@rule
def my_rule(zip_binary: ZipBinary) -> MyOutput:
return MyOutput()

@rule
async def my_rule_lazy() -> MyOutput:
zip_binary = await Get(ZipBinary, ZipBinaryRequest())
return MyOutput()
```

The lazy API is useful, for example, when you only want to `Get` that output type inside an `if` branch.

We added syntax in 2.17 to now use `Get(OutputType)`, whereas before you had to do `Get(OutputType, OutputTypeRequest)` or (as of 2.15) `Get(OutputType, {})`. So, these `OutputTypeRequest` types are now redudent and deprecated in favor of simply using `Get(OutputType)`.


2.16
----

Expand Down
11 changes: 3 additions & 8 deletions pants-plugins/internal_plugins/test_lockfile_fixtures/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,10 @@ class RenderedJVMLockfileFixtures(DeduplicatedCollection[RenderedJVMLockfileFixt
pass


@dataclass(frozen=True)
class CollectFixtureConfigsRequest:
pass


# TODO: This rule was mostly copied from the rule `setup_pytest_for_target` in
# `src/python/pants/backend/python/goals/pytest_runner.py`. Some refactoring should be done.
@rule
async def collect_fixture_configs(
_request: CollectFixtureConfigsRequest,
pytest: PyTest,
python_setup: PythonSetup,
test_extra_env: TestExtraEnv,
Expand Down Expand Up @@ -177,8 +171,9 @@ async def collect_fixture_configs(


@rule
async def gather_lockfile_fixtures() -> RenderedJVMLockfileFixtures:
configs = await Get(CollectedJVMLockfileFixtureConfigs, CollectFixtureConfigsRequest())
async def gather_lockfile_fixtures(
configs: CollectedJVMLockfileFixtureConfigs,
) -> RenderedJVMLockfileFixtures:
rendered_fixtures = []
for config in configs:
artifact_reqs = ArtifactRequirements(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,7 @@
)
from pants.core.goals.fmt import FmtResult, FmtTargetsRequest, Partitions
from pants.core.util_rules.external_tool import DownloadedExternalTool, ExternalToolRequest
from pants.core.util_rules.system_binaries import (
BinaryShims,
BinaryShimsRequest,
DiffBinary,
DiffBinaryRequest,
)
from pants.core.util_rules.system_binaries import BinaryShims, BinaryShimsRequest, DiffBinary
from pants.engine.fs import Digest, MergeDigests
from pants.engine.platform import Platform
from pants.engine.process import Process, ProcessResult
Expand Down Expand Up @@ -62,9 +57,8 @@ async def partition_buf(

@rule(desc="Format with buf format", level=LogLevel.DEBUG)
async def run_buf_format(
request: BufFormatRequest.Batch, buf: BufSubsystem, platform: Platform
request: BufFormatRequest.Batch, buf: BufSubsystem, diff_binary: DiffBinary, platform: Platform
) -> FmtResult:
diff_binary = await Get(DiffBinary, DiffBinaryRequest())
download_buf_get = Get(DownloadedExternalTool, ExternalToolRequest, buf.get_request(platform))
binary_shims_get = Get(
BinaryShims,
Expand Down
23 changes: 13 additions & 10 deletions src/python/pants/backend/docker/util_rules/docker_binary.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from pants.backend.docker.subsystems.docker_options import DockerOptions
from pants.backend.docker.util_rules.docker_build_args import DockerBuildArgs
from pants.base.deprecated import warn_or_error
from pants.core.util_rules.system_binaries import (
BinaryPath,
BinaryPathRequest,
Expand Down Expand Up @@ -120,14 +121,21 @@ def run_image(

@dataclass(frozen=True)
class DockerBinaryRequest:
pass
def __post_init__(self) -> None:
warn_or_error(
"2.18.0.dev0",
"using `Get(DockerBinary, DockerBinaryRequest)",
"Instead, simply use `Get(DockerBinary)` or put `DockerBinary` in the rule signature",
)


async def find_docker(_: DockerBinaryRequest, docker: DockerBinary) -> DockerBinary:
return docker


@rule(desc="Finding the `docker` binary and related tooling", level=LogLevel.DEBUG)
async def find_docker(
docker_request: DockerBinaryRequest,
docker_options: DockerOptions,
docker_options_env_aware: DockerOptions.EnvironmentAware,
async def get_docker(
docker_options: DockerOptions, docker_options_env_aware: DockerOptions.EnvironmentAware
) -> DockerBinary:
env = await Get(EnvironmentVars, EnvironmentVarsRequest(["PATH"]))
search_path = docker_options_env_aware.executable_search_path(env)
Expand Down Expand Up @@ -162,10 +170,5 @@ async def find_docker(
)


@rule
async def get_docker() -> DockerBinary:
return await Get(DockerBinary, DockerBinaryRequest())


def rules():
return collect_rules()
64 changes: 37 additions & 27 deletions src/python/pants/backend/go/util_rules/cgo.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,9 @@
from pants.backend.go.util_rules.cgo_security import check_linker_flags
from pants.backend.go.util_rules.goroot import GoRoot
from pants.backend.go.util_rules.sdk import GoSdkProcess
from pants.base.deprecated import warn_or_error
from pants.base.glob_match_error_behavior import GlobMatchErrorBehavior
from pants.core.util_rules.system_binaries import (
BashBinary,
BashBinaryRequest,
BinaryPath,
BinaryPathTest,
)
from pants.core.util_rules.system_binaries import BashBinary, BinaryPath, BinaryPathTest
from pants.engine.engine_aware import EngineAwareParameter
from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest
from pants.engine.fs import (
Expand Down Expand Up @@ -293,7 +289,15 @@ async def setup_compiler_cmd(

@dataclass(frozen=True)
class CGoCompilerWrapperScriptRequest:
pass
def __post_init__(self) -> None:
warn_or_error(
"2.18.0.dev0",
"using `Get(CGoCompilerWrapperScript, CGoCompilerWrapperScriptRequest)",
(
"Instead, simply use `Get(CGoCompilerWrapperScript)` or put "
+ "`CGoCompilerWrapperScript` in the rule signature"
),
)


@dataclass(frozen=True)
Expand All @@ -302,9 +306,7 @@ class CGoCompilerWrapperScript:


@rule
async def make_cgo_compile_wrapper_script(
_: CGoCompilerWrapperScriptRequest,
) -> CGoCompilerWrapperScript:
async def make_cgo_compile_wrapper_script() -> CGoCompilerWrapperScript:
digest = await Get(
Digest,
CreateDigest(
Expand All @@ -326,6 +328,13 @@ async def make_cgo_compile_wrapper_script(
return CGoCompilerWrapperScript(digest=digest)


@rule
def cgo_wrapper_compile_script_request(
_: CGoCompilerWrapperScriptRequest, script: CGoCompilerWrapperScript
) -> CGoCompilerWrapperScript:
return script


async def _cc(
binary_name: str,
input_digest: Digest,
Expand All @@ -336,15 +345,17 @@ async def _cc(
description: str,
golang_env_aware: GolangSubsystem.EnvironmentAware,
) -> Process:
compiler_path = await Get(
BinaryPath,
CGoBinaryPathRequest(
binary_name=binary_name,
binary_path_test=BinaryPathTest(["--version"]),
compiler_path, bash, wrapper_script = await MultiGet(
Get(
BinaryPath,
CGoBinaryPathRequest(
binary_name=binary_name,
binary_path_test=BinaryPathTest(["--version"]),
),
),
Get(BashBinary),
Get(CGoCompilerWrapperScript),
)
bash = await Get(BashBinary, BashBinaryRequest())
wrapper_script = await Get(CGoCompilerWrapperScript, CGoCompilerWrapperScriptRequest())
compiler_args_result, env, input_digest = await MultiGet(
Get(SetupCompilerCmdResult, SetupCompilerCmdRequest((compiler_path.path,), dir_path)),
Get(
Expand Down Expand Up @@ -383,17 +394,16 @@ async def _gccld(
objs: Iterable[str],
description: str,
) -> FallibleProcessResult:
compiler_path = await Get(
BinaryPath,
CGoBinaryPathRequest(
binary_name=binary_name,
binary_path_test=BinaryPathTest(["--version"]),
compiler_path, bash, wrapper_script = await MultiGet(
Get(
BinaryPath,
CGoBinaryPathRequest(
binary_name=binary_name,
binary_path_test=BinaryPathTest(["--version"]),
),
),
)

bash, wrapper_script = await MultiGet(
Get(BashBinary, BashBinaryRequest()),
Get(CGoCompilerWrapperScript, CGoCompilerWrapperScriptRequest()),
Get(BashBinary),
Get(CGoCompilerWrapperScript),
)

compiler_args_result, env, input_digest = await MultiGet(
Expand Down
5 changes: 2 additions & 3 deletions src/python/pants/backend/python/dependency_inference/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.base.glob_match_error_behavior import GlobMatchErrorBehavior
from pants.core import target_types
from pants.core.target_types import AllAssetTargets, AllAssetTargetsByPath, AllAssetTargetsRequest
from pants.core.target_types import AllAssetTargetsByPath
from pants.core.util_rules import stripped_source_files
from pants.engine.addresses import Address, Addresses
from pants.engine.internals.graph import Owners, OwnersRequest
Expand Down Expand Up @@ -430,8 +430,7 @@ async def resolve_parsed_dependencies(
resolve_results = {}

if parsed_assets:
all_asset_targets = await Get(AllAssetTargets, AllAssetTargetsRequest())
assets_by_path = await Get(AllAssetTargetsByPath, AllAssetTargets, all_asset_targets)
assets_by_path = await Get(AllAssetTargetsByPath)
asset_deps = _get_inferred_asset_deps(
request.field_set.address,
request.field_set.source.file_path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
PythonDependencyVisitorRequest,
get_scripts_digest,
)
from pants.backend.python.framework.django.detect_apps import DjangoApps, DjangoAppsRequest
from pants.backend.python.framework.django.detect_apps import DjangoApps
from pants.engine.fs import CreateDigest, FileContent
from pants.engine.internals.native_engine import Digest, MergeDigests
from pants.engine.internals.selectors import Get
Expand All @@ -28,8 +28,8 @@ class DjangoDependencyVisitorRequest(PythonDependencyVisitorRequest):
@rule
async def django_parser_script(
_: DjangoDependencyVisitorRequest,
django_apps: DjangoApps,
) -> PythonDependencyVisitor:
django_apps = await Get(DjangoApps, DjangoAppsRequest())
django_apps_digest = await Get(
Digest, CreateDigest([FileContent("apps.json", django_apps.to_json())])
)
Expand Down
18 changes: 13 additions & 5 deletions src/python/pants/backend/python/framework/django/detect_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from pants.backend.python.target_types import InterpreterConstraintsField
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.backend.python.util_rules.pex_environment import PythonExecutable
from pants.base.deprecated import warn_or_error
from pants.base.specs import FileGlobSpec, RawSpecs
from pants.engine.fs import AddPrefix, Digest, MergeDigests
from pants.engine.internals.selectors import Get, MultiGet
Expand All @@ -28,7 +29,12 @@

@dataclass(frozen=True)
class DjangoAppsRequest:
pass
def __post_init__(self) -> None:
warn_or_error(
"2.18.0.dev0",
"using `Get(DjangoApps, DjangoAppsRequest)",
"Instead, simply use `Get(DjangoApps)` or put `DjangoApps` in the rule signature",
)


@dataclass(frozen=True)
Expand All @@ -44,10 +50,7 @@ def to_json(self) -> bytes:


@rule
async def detect_django_apps(
_: DjangoAppsRequest,
python_setup: PythonSetup,
) -> DjangoApps:
async def detect_django_apps(python_setup: PythonSetup) -> DjangoApps:
# A Django app has a "name" - the full import path to the app ("path.to.myapp"),
# and a "label" - a short name, usually the last segment of the import path ("myapp").
#
Expand Down Expand Up @@ -130,6 +133,11 @@ async def detect_django_apps(
return django_apps


@rule
def django_apps_from_request(_: DjangoAppsRequest, django_apps: DjangoApps) -> DjangoApps:
return django_apps


def rules():
return [
*collect_rules(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from pants.backend.python.dependency_inference import parse_python_dependencies
from pants.backend.python.framework.django import dependency_inference, detect_apps
from pants.backend.python.framework.django.detect_apps import DjangoApps, DjangoAppsRequest
from pants.backend.python.framework.django.detect_apps import DjangoApps
from pants.backend.python.target_types import PythonSourceTarget
from pants.backend.python.util_rules import pex
from pants.core.util_rules import stripped_source_files
Expand Down Expand Up @@ -35,7 +35,7 @@ def rule_runner() -> RuleRunner:
*pex.rules(),
*dependency_inference.rules(),
*detect_apps.rules(),
QueryRule(DjangoApps, [DjangoAppsRequest, EnvironmentName]),
QueryRule(DjangoApps, [EnvironmentName]),
],
target_types=[PythonSourceTarget],
)
Expand Down Expand Up @@ -112,7 +112,7 @@ class App3AppConfig(AppConfig):
)
result = rule_runner.request(
DjangoApps,
[DjangoAppsRequest()],
[],
)
assert result == DjangoApps(
FrozenDict({"app1": "path.to.app1", "app2_label": "another.path.app2", "app3": "some.app3"})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule
from pants.engine.target import (
AllTargets,
AllTargetsRequest,
RegisteredTargetTypes,
TransitiveTargets,
TransitiveTargetsRequest,
Expand Down Expand Up @@ -80,7 +79,7 @@ async def py_constraints(
)
return PyConstraintsGoal(exit_code=1)

all_targets = await Get(AllTargets, AllTargetsRequest())
all_targets = await Get(AllTargets)
all_python_targets = tuple(
t for t in all_targets if t.has_field(InterpreterConstraintsField)
)
Expand Down
3 changes: 1 addition & 2 deletions src/python/pants/backend/python/typecheck/mypy/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
from pants.engine.rules import Get, collect_rules, rule
from pants.engine.target import (
AllTargets,
AllTargetsRequest,
FieldSet,
Target,
TransitiveTargets,
Expand Down Expand Up @@ -415,7 +414,7 @@ async def setup_mypy_extra_type_stubs_lockfile(
#
# This first finds the ICs of each partition. Then, it ORs all unique resulting interpreter
# constraints. The net effect is that every possible Python interpreter used will be covered.
all_tgts = await Get(AllTargets, AllTargetsRequest())
all_tgts = await Get(AllTargets)
all_field_sets = [
MyPyFieldSet.create(tgt) for tgt in all_tgts if MyPyFieldSet.is_applicable(tgt)
]
Expand Down
Loading

0 comments on commit 02e0ba2

Please sign in to comment.