Skip to content

Commit

Permalink
Remove deprecated v1 Subsystem facilities (pantsbuild#11424)
Browse files Browse the repository at this point in the history
### Problem

As initiated in pantsbuild#11388, the v1 `Subsystem.{global_instance,scoped_instance}()` facilities are being removed.

### Solution

Remove `Subsystem.{global_instance,scoped_instance}()` and their support code. To reduce bootstrap dependencies, the `RunTracker` is no longer a `Subsystem`: its options have moved to being global options, and are deprecated in their previous locations. The `--streaming-workunit-handlers` option (which relied on singleton `Subsystem`s) is also removed in favor of the APIs added in pantsbuild#11388.

### Result

Plugins that need access to `Subsystem`s should do so via `@rule` arguments, and callers outside of `@rule`s should use `Scheduler.product_request(<subsystem_cls>, [Params()])` to get an instance.
  • Loading branch information
stuhood authored Jan 7, 2021
1 parent 4152d6f commit 9cde7c0
Show file tree
Hide file tree
Showing 16 changed files with 201 additions and 356 deletions.
1 change: 0 additions & 1 deletion build-support/bin/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ function execute_packaged_pants_with_internal_backends() {
--no-verify-config \
--no-pantsd \
--pythonpath="['pants-plugins']" \
--streaming-workunits-handlers="[]" \
--backend-packages="[\
'pants.backend.awslambda.python',\
'pants.backend.python',\
Expand Down
41 changes: 19 additions & 22 deletions src/python/pants/bin/local_pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
from pants.option.errors import UnknownFlagsError
from pants.option.options import Options
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.option.subsystem import Subsystem
from pants.util.contextutil import maybe_profiled

logger = logging.getLogger(__name__)
Expand All @@ -50,6 +49,7 @@ class LocalPantsRunner:
build_root: The build root for this run.
options: The parsed options for this run.
build_config: The parsed build configuration for this run.
run_tracker: A tracker for metrics for the run.
specs: The specs for this run, i.e. either the address or filesystem specs.
graph_session: A LegacyGraphSession instance for graph reuse.
profile_path: The profile path - if any (from from the `PANTS_PROFILE` env var).
Expand All @@ -58,6 +58,7 @@ class LocalPantsRunner:
build_root: str
options: Options
build_config: BuildConfiguration
run_tracker: RunTracker
specs: Specs
graph_session: GraphSession
union_membership: UnionMembership
Expand All @@ -68,6 +69,7 @@ def _init_graph_session(
cls,
options_bootstrapper: OptionsBootstrapper,
build_config: BuildConfiguration,
run_tracker: RunTracker,
options: Options,
scheduler: Optional[GraphScheduler] = None,
cancellation_latch: Optional[PySessionCancellationLatch] = None,
Expand All @@ -85,7 +87,7 @@ def _init_graph_session(
raise

return graph_scheduler_helper.new_session(
RunTracker.global_instance(silent=True).run_id,
run_tracker.run_id,
dynamic_ui=global_scope.dynamic_ui,
use_colors=global_scope.get("colors", True),
session_values=SessionValues(
Expand Down Expand Up @@ -133,12 +135,13 @@ def create(
cls._handle_unknown_flags(err, options_bootstrapper)
raise

run_tracker = RunTracker(options)
union_membership = UnionMembership.from_rules(build_config.union_rules)

# If we're running with the daemon, we'll be handed a warmed Scheduler, which we use
# to initialize a session here.
graph_session = cls._init_graph_session(
options_bootstrapper, build_config, options, scheduler, cancellation_latch
options_bootstrapper, build_config, run_tracker, options, scheduler, cancellation_latch
)

# Option values are usually computed lazily on demand,
Expand Down Expand Up @@ -167,19 +170,20 @@ def create(
build_root=build_root,
options=options,
build_config=build_config,
run_tracker=run_tracker,
specs=specs,
graph_session=graph_session,
union_membership=union_membership,
profile_path=profile_path,
)

def _start_run(self, run_tracker: RunTracker, start_time: float) -> None:
run_tracker.start(self.options, run_start_time=start_time)
def _start_run(self, start_time: float) -> None:
self.run_tracker.start(run_start_time=start_time)

spec_parser = SpecsParser(get_buildroot())
specs = [str(spec_parser.parse_spec(spec)) for spec in self.options.specs]
# Note: This will not include values from `--changed-*` flags.
run_tracker.run_info.add_info("specs_from_command_line", specs, stringify=False)
self.run_tracker.run_info.add_info("specs_from_command_line", specs, stringify=False)

def _run_v2(self, goals: Tuple[str, ...]) -> ExitCode:
if not goals:
Expand Down Expand Up @@ -211,12 +215,12 @@ def _maybe_run_v2_body(self, goals, poll: bool) -> ExitCode:
poll_delay=(0.1 if poll else None),
)

def _finish_run(self, run_tracker: RunTracker, code: ExitCode) -> None:
def _finish_run(self, code: ExitCode) -> None:
"""Cleans up the run tracker."""

metrics = self.graph_session.scheduler_session.metrics()
run_tracker.set_pantsd_scheduler_metrics(metrics)
run_tracker.end_run(code)
self.run_tracker.set_pantsd_scheduler_metrics(metrics)
self.run_tracker.end_run(code)

def _print_help(self, request: HelpRequest) -> ExitCode:
global_options = self.options.for_global_scope()
Expand All @@ -236,22 +240,15 @@ def _print_help(self, request: HelpRequest) -> ExitCode:
return help_printer.print_help()

def _get_workunits_callbacks(self) -> Tuple[WorkunitsCallback, ...]:
# Load WorkunitsCallbacks with the legacy `get_streaming_workunit_callbacks` method, and with
# the new WorkunitsCallbackFactories method.
# NB: When the `--streaming-workunits-handlers` deprecation triggers, the first method will be
# removed.
streaming_handlers = self.options.for_global_scope().streaming_workunits_handlers
callbacks = list(Subsystem.get_streaming_workunit_callbacks(streaming_handlers))
# Load WorkunitsCallbacks by requesting WorkunitsCallbackFactories, and then constructing
# a per-run instance of each WorkunitsCallback.
(workunits_callback_factories,) = self.graph_session.scheduler_session.product_request(
WorkunitsCallbackFactories, [self.union_membership]
)
for wcf in workunits_callback_factories:
callbacks.append(wcf.callback_factory())
return tuple(callbacks)
return tuple(wcf.callback_factory() for wcf in workunits_callback_factories)

def run(self, start_time: float) -> ExitCode:
run_tracker = RunTracker.global_instance(silent=True)
self._start_run(run_tracker, start_time)
self._start_run(start_time)

with maybe_profiled(self.profile_path):
global_options = self.options.for_global_scope()
Expand All @@ -261,7 +258,7 @@ def run(self, start_time: float) -> ExitCode:

streaming_reporter = StreamingWorkunitHandler(
self.graph_session.scheduler_session,
run_tracker=run_tracker,
run_tracker=self.run_tracker,
callbacks=self._get_workunits_callbacks(),
report_interval_seconds=global_options.streaming_workunits_report_interval,
)
Expand All @@ -274,5 +271,5 @@ def run(self, start_time: float) -> ExitCode:
except Exception as e:
ExceptionSink.log_exception(e)

self._finish_run(run_tracker, engine_result)
self._finish_run(engine_result)
return engine_result
4 changes: 2 additions & 2 deletions src/python/pants/build_graph/build_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from pants.engine.rules import Rule, RuleIndex
from pants.engine.target import Target
from pants.engine.unions import UnionRule
from pants.goal.run_tracker import RunTracker
from pants.goal.run_tracker import DeprecatedRunTracker
from pants.option.global_options import GlobalOptions
from pants.option.optionable import Optionable
from pants.option.scope import normalize_scope
Expand All @@ -33,7 +33,7 @@

# Subsystems used outside of any rule.
_GLOBAL_SUBSYSTEMS: FrozenOrderedSet[Type[Optionable]] = FrozenOrderedSet(
{GlobalOptions, RunTracker, Changed}
{GlobalOptions, DeprecatedRunTracker, Changed}
)


Expand Down
10 changes: 10 additions & 0 deletions src/python/pants/engine/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,15 @@ python_library()
python_tests(
name='tests',
dependencies=['src/python/pants/engine/internals:fs_test_data'],
sources=["*_test.py", "!*_integration_test.py"],
timeout=90,
)

python_integration_tests(
name='integration_tests',
uses_pants_run=True,
dependencies=[
# Loaded reflectively as a backend in `streaming_workunit_handler_integration_test.py`.
'testprojects/pants-plugins/src/python/workunit_logger',
],
)
32 changes: 24 additions & 8 deletions src/python/pants/engine/internals/engine_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
StreamingWorkunitHandler,
)
from pants.goal.run_tracker import RunTracker
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.rule_runner import QueryRule, RuleRunner
from pants.util.logging import LogLevel

Expand Down Expand Up @@ -277,6 +278,17 @@ def add(self, **kwargs) -> None:
self.finished_workunit_chunks.append(completed_workunits)


def new_run_tracker() -> RunTracker:
# NB: A RunTracker usually observes "all options" (`get_full_options`), but it only actually
# directly consumes bootstrap options.
return RunTracker(create_options_bootstrapper([]).bootstrap_options)


@pytest.fixture
def run_tracker() -> RunTracker:
return new_run_tracker()


class StreamingWorkunitTests(unittest.TestCase, SchedulerTestBase):
def _fixture_for_rules(
self, rules, max_workunit_verbosity: LogLevel = LogLevel.INFO
Expand All @@ -286,7 +298,7 @@ def _fixture_for_rules(
tracker = WorkunitTracker()
handler = StreamingWorkunitHandler(
scheduler,
run_tracker=RunTracker("run-tracker", None),
run_tracker=new_run_tracker(),
callbacks=[tracker.add],
report_interval_seconds=0.01,
max_workunit_verbosity=max_workunit_verbosity,
Expand Down Expand Up @@ -644,11 +656,11 @@ def rule_runner() -> RuleRunner:
)


def test_more_complicated_engine_aware(rule_runner: RuleRunner) -> None:
def test_more_complicated_engine_aware(rule_runner: RuleRunner, run_tracker: RunTracker) -> None:
tracker = WorkunitTracker()
handler = StreamingWorkunitHandler(
rule_runner.scheduler,
run_tracker=RunTracker("run-tracker", None),
run_tracker=run_tracker,
callbacks=[tracker.add],
report_interval_seconds=0.01,
max_workunit_verbosity=LogLevel.TRACE,
Expand Down Expand Up @@ -699,13 +711,15 @@ def test_more_complicated_engine_aware(rule_runner: RuleRunner) -> None:
assert len(tuple(x for x in digest_contents_2 if x.content == b"gamma")) == 1


def test_process_digests_on_streaming_workunits(rule_runner: RuleRunner) -> None:
def test_process_digests_on_streaming_workunits(
rule_runner: RuleRunner, run_tracker: RunTracker
) -> None:
scheduler = rule_runner.scheduler

tracker = WorkunitTracker()
handler = StreamingWorkunitHandler(
scheduler,
run_tracker=RunTracker("run-tracker", None),
run_tracker=run_tracker,
callbacks=[tracker.add],
report_interval_seconds=0.01,
max_workunit_verbosity=LogLevel.INFO,
Expand Down Expand Up @@ -735,7 +749,7 @@ def test_process_digests_on_streaming_workunits(rule_runner: RuleRunner) -> None
tracker = WorkunitTracker()
handler = StreamingWorkunitHandler(
scheduler,
run_tracker=RunTracker("run-tracker", None),
run_tracker=run_tracker,
callbacks=[tracker.add],
report_interval_seconds=0.01,
max_workunit_verbosity=LogLevel.INFO,
Expand Down Expand Up @@ -776,7 +790,9 @@ def test_process_digests_on_streaming_workunits(rule_runner: RuleRunner) -> None
assert byte_outputs[1] == result.stderr


def test_context_object_on_streaming_workunits(rule_runner: RuleRunner) -> None:
def test_context_object_on_streaming_workunits(
rule_runner: RuleRunner, run_tracker: RunTracker
) -> None:
scheduler = rule_runner.scheduler

def callback(**kwargs) -> None:
Expand All @@ -792,7 +808,7 @@ def callback(**kwargs) -> None:

handler = StreamingWorkunitHandler(
scheduler,
run_tracker=RunTracker("run-tracker", None),
run_tracker=run_tracker,
callbacks=[callback],
report_interval_seconds=0.01,
max_workunit_verbosity=LogLevel.INFO,
Expand Down
4 changes: 1 addition & 3 deletions src/python/pants/engine/internals/options_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ def __post_init__(self) -> None:

@memoized_property
def options(self) -> Options:
return OptionsInitializer.create(
self.options_bootstrapper, self.build_config, init_subsystems=False
)
return OptionsInitializer.create(self.options_bootstrapper, self.build_config)


@rule
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import os

from pants.testutil.pants_integration_test import run_pants, setup_tmpdir
from pants.util.dirutil import maybe_read_file


def test_workunits_logger() -> None:
with setup_tmpdir({}) as tmpdir:
dest = os.path.join(tmpdir, "dest.log")
pants_run = run_pants(
[
"--backend-packages=+['workunit_logger','pants.backend.python']",
f"--workunit-logger-dest={dest}",
"list",
"3rdparty::",
]
)
pants_run.assert_success()
# Assert that the file was created and non-empty.
assert maybe_read_file(dest)
Loading

0 comments on commit 9cde7c0

Please sign in to comment.