From 72c4b93bb75216f62bf222ba4a80a43b924f5d74 Mon Sep 17 00:00:00 2001 From: gshuflin Date: Thu, 19 Mar 2020 10:51:40 -0700 Subject: [PATCH] Make LocalPantsRunner a dataclass (#9339) This commit attempts to make the code in LocalPantsRunner cleaner by turning it into a dataclass, which allows us to get rid of the `__init__` method. Some single-use methods and properties on `LocalPantsRunner` have also been made more concise, and a number of type annotations were added to it and in several other places. `local_pants_runner.py` is now about 30 lines shorter --- src/python/pants/base/exceptions.py | 4 +- src/python/pants/bin/goal_runner.py | 8 +- src/python/pants/bin/local_pants_runner.py | 179 +++++++++----------- src/python/pants/bin/remote_pants_runner.py | 2 +- src/python/pants/engine/legacy_engine.py | 4 +- src/python/pants/engine/scheduler.py | 6 +- src/python/pants/init/engine_initializer.py | 2 +- 7 files changed, 90 insertions(+), 115 deletions(-) diff --git a/src/python/pants/base/exceptions.py b/src/python/pants/base/exceptions.py index f1a41c624b3..2d39a1a48cf 100644 --- a/src/python/pants/base/exceptions.py +++ b/src/python/pants/base/exceptions.py @@ -15,12 +15,12 @@ def __init__(self, *args, **kwargs): :param int exit_code: an optional exit code (defaults to nonzero) :param list failed_targets: an optional list of failed targets (default=[]) """ - self._exit_code = kwargs.pop("exit_code", PANTS_FAILED_EXIT_CODE) + self._exit_code: int = kwargs.pop("exit_code", PANTS_FAILED_EXIT_CODE) self._failed_targets = kwargs.pop("failed_targets", []) super().__init__(*args, **kwargs) @property - def exit_code(self): + def exit_code(self) -> int: return self._exit_code @property diff --git a/src/python/pants/bin/goal_runner.py b/src/python/pants/bin/goal_runner.py index 7fa8368f853..19c2062df3b 100644 --- a/src/python/pants/bin/goal_runner.py +++ b/src/python/pants/bin/goal_runner.py @@ -140,7 +140,7 @@ def _setup_context(self): return goals, context - def create(self): + def create(self) -> "GoalRunner": goals, context = self._setup_context() return GoalRunner( context=context, @@ -182,7 +182,7 @@ def _is_valid_workdir(self, workdir): ) return False - def _execute_engine(self): + def _execute_engine(self) -> int: engine = RoundEngine() sorted_goal_infos = engine.sort_goals(self._context, self._goals) RunTracker.global_instance().set_sorted_goal_infos(sorted_goal_infos) @@ -193,7 +193,7 @@ def _execute_engine(self): return result - def _run_goals(self): + def _run_goals(self) -> int: should_kill_nailguns = self._kill_nailguns try: @@ -221,7 +221,7 @@ def _run_goals(self): return result - def run(self): + def run(self) -> int: global_options = self._context.options.for_global_scope() if not self._is_valid_workdir(global_options.pants_workdir): diff --git a/src/python/pants/bin/local_pants_runner.py b/src/python/pants/bin/local_pants_runner.py index 7f285045fcf..99858e91b9e 100644 --- a/src/python/pants/bin/local_pants_runner.py +++ b/src/python/pants/bin/local_pants_runner.py @@ -4,6 +4,7 @@ import logging import os from contextlib import contextmanager +from dataclasses import dataclass from typing import List, Mapping, Optional, Tuple from pants.base.build_environment import get_buildroot @@ -22,7 +23,7 @@ from pants.init.engine_initializer import EngineInitializer, LegacyGraphSession from pants.init.logging import setup_logging_from_options from pants.init.options_initializer import BuildConfigInitializer, OptionsInitializer -from pants.init.repro import Reproducer +from pants.init.repro import Repro, Reproducer from pants.init.specs_calculator import SpecsCalculator from pants.option.arg_splitter import UnknownGoalHelp from pants.option.options import Options @@ -96,8 +97,34 @@ def exit(self, result=PANTS_SUCCEEDED_EXIT_CODE, msg=None, *args, **kwargs): super().exit(result=result, msg=msg, *args, **kwargs) +@dataclass class LocalPantsRunner(ExceptionSink.AccessGlobalExiterMixin): - """Handles a single pants invocation running in the process-local context.""" + """Handles a single pants invocation running in the process-local context. + + build_root: The build root for this run. + options: The parsed options for this run. + options_bootstrapper: The OptionsBootstrapper instance to use. + build_config: The parsed build configuration for this run. + specs: The specs for this run, i.e. either the address or filesystem specs. + graph_session: A LegacyGraphSession instance for graph reuse. + is_daemon: Whether or not this run was launched with a daemon graph helper. + profile_path: The profile path - if any (from from the `PANTS_PROFILE` env var). + """ + + build_root: str + options: Options + options_bootstrapper: OptionsBootstrapper + build_config: BuildConfiguration + specs: Specs + graph_session: LegacyGraphSession + scheduler_session: SchedulerSession + union_membership: UnionMembership + is_daemon: bool + profile_path: Optional[str] + _run_start_time: Optional[float] = None + _run_tracker: Optional[RunTracker] = None + _reporting: Optional[Reporting] = None + _repro: Optional[Repro] = None @staticmethod def parse_options( @@ -142,25 +169,6 @@ def _maybe_init_graph_session( ) return graph_session, graph_session.scheduler_session - @staticmethod - def _maybe_init_specs( - specs: Optional[Specs], - graph_session: LegacyGraphSession, - options: Options, - build_root: str, - ) -> Specs: - if specs: - return specs - - global_options = options.for_global_scope() - return SpecsCalculator.create( - options=options, - build_root=build_root, - session=graph_session.scheduler_session, - exclude_patterns=tuple(global_options.exclude_target_regexp), - tags=tuple(global_options.tag), - ) - @classmethod def create( cls, @@ -205,7 +213,15 @@ def create( daemon_graph_session, options_bootstrapper, build_config, options ) - specs = cls._maybe_init_specs(specs, graph_session, options, build_root) + if specs is None: + global_options = options.for_global_scope() + specs = SpecsCalculator.create( + options=options, + build_root=build_root, + session=graph_session.scheduler_session, + exclude_patterns=tuple(global_options.exclude_target_regexp), + tags=tuple(global_options.tag), + ) profile_path = env.get("PANTS_PROFILE") @@ -222,47 +238,7 @@ def create( profile_path=profile_path, ) - def __init__( - self, - build_root: str, - options: Options, - options_bootstrapper: OptionsBootstrapper, - build_config: BuildConfiguration, - specs: Specs, - graph_session: LegacyGraphSession, - scheduler_session: SchedulerSession, - union_membership: UnionMembership, - is_daemon: bool, - profile_path: Optional[str], - ) -> None: - """ - :param build_root: The build root for this run. - :param options: The parsed options for this run. - :param options_bootstrapper: The OptionsBootstrapper instance to use. - :param build_config: The parsed build configuration for this run. - :param specs: The specs for this run, i.e. either the address or filesystem specs. - :param graph_session: A LegacyGraphSession instance for graph reuse. - :param is_daemon: Whether or not this run was launched with a daemon graph helper. - :param profile_path: The profile path - if any (from from the `PANTS_PROFILE` env var). - """ - self._build_root = build_root - self._options = options - self._options_bootstrapper = options_bootstrapper - self._build_config = build_config - self._specs = specs - self._graph_session = graph_session - self._scheduler_session = scheduler_session - self._union_membership = union_membership - self._is_daemon = is_daemon - self._profile_path = profile_path - - self._run_start_time = None - self._run_tracker = None - self._reporting = None - self._repro = None - self._global_options = options.for_global_scope() - - def set_start_time(self, start_time): + def set_start_time(self, start_time: float) -> None: # Launch RunTracker as early as possible (before .run() is called). self._run_tracker = RunTracker.global_instance() @@ -272,12 +248,10 @@ def set_start_time(self, start_time): self._reporting = Reporting.global_instance() self._run_start_time = start_time - self._reporting.initialize( - self._run_tracker, self._options, start_time=self._run_start_time - ) + self._reporting.initialize(self._run_tracker, self.options, start_time=self._run_start_time) spec_parser = CmdLineSpecParser(get_buildroot()) - specs = [spec_parser.parse_spec(spec).to_spec_string() for spec in self._options.specs] + specs = [spec_parser.parse_spec(spec).to_spec_string() for spec in self.options.specs] # Note: This will not include values from `--changed-*` flags. self._run_tracker.run_info.add_info("specs_from_command_line", specs, stringify=False) @@ -288,27 +262,25 @@ def set_start_time(self, start_time): def run(self): with LocalExiter.wrap_global_exiter(self._run_tracker, self._repro), maybe_profiled( - self._profile_path + self.profile_path ): self._run() def _maybe_handle_help(self): """Handle requests for `help` information.""" - if self._options.help_request: - help_printer = HelpPrinter( - options=self._options, union_membership=self._union_membership - ) + if self.options.help_request: + help_printer = HelpPrinter(options=self.options, union_membership=self.union_membership) result = help_printer.print_help() return result - def _maybe_run_v1(self): - v1_goals, ambiguous_goals, _ = self._options.goals_by_version - if not self._global_options.v1: + def _maybe_run_v1(self, v1: bool) -> int: + v1_goals, ambiguous_goals, _ = self.options.goals_by_version + if not v1: if v1_goals: HelpPrinter( - options=self._options, - help_request=UnknownGoalHelp(v1_goals), - union_membership=self._union_membership, + options=self.options, + help_request=UnknownGoalHelp(list(v1_goals)), + union_membership=self.union_membership, ).print_help() return PANTS_FAILED_EXIT_CODE return PANTS_SUCCEEDED_EXIT_CODE @@ -319,38 +291,39 @@ def _maybe_run_v1(self): # Setup and run GoalRunner. return ( GoalRunner.Factory( - self._build_root, - self._options_bootstrapper, - self._options, - self._build_config, - self._run_tracker, - self._reporting, - self._graph_session, - self._specs, + self.build_root, + self.options_bootstrapper, + self.options, + self.build_config, + self._run_tracker, # type: ignore + self._reporting, # type: ignore + self.graph_session, + self.specs, self._exiter, ) .create() .run() ) - def _maybe_run_v2(self): + def _maybe_run_v2(self, v2: bool) -> int: # N.B. For daemon runs, @goal_rules are invoked pre-fork - # so this path only serves the non-daemon run mode. - if self._is_daemon: + if self.is_daemon: return PANTS_SUCCEEDED_EXIT_CODE - _, ambiguous_goals, v2_goals = self._options.goals_by_version - goals = v2_goals + (ambiguous_goals if self._global_options.v2 else tuple()) - self._run_tracker.set_v2_goal_rule_names(goals) + _, ambiguous_goals, v2_goals = self.options.goals_by_version + goals = v2_goals + (ambiguous_goals if v2 else tuple()) + if self._run_tracker: + self._run_tracker.set_v2_goal_rule_names(goals) if not goals: return PANTS_SUCCEEDED_EXIT_CODE - return self._graph_session.run_goal_rules( - options_bootstrapper=self._options_bootstrapper, - union_membership=self._union_membership, - options=self._options, + return self.graph_session.run_goal_rules( + options_bootstrapper=self.options_bootstrapper, + union_membership=self.union_membership, + options=self.options, goals=goals, - specs=self._specs, + specs=self.specs, ) @staticmethod @@ -363,30 +336,32 @@ def _compute_final_exit_code(*codes): return max_code def _update_stats(self): - metrics = self._scheduler_session.metrics() + metrics = self.scheduler_session.metrics() self._run_tracker.pantsd_stats.set_scheduler_metrics(metrics) - engine_workunits = self._scheduler_session.engine_workunits(metrics) + engine_workunits = self.scheduler_session.engine_workunits(metrics) if engine_workunits: self._run_tracker.report.bulk_record_workunits(engine_workunits) def _run(self): - global_options = self._options.for_global_scope() + global_options = self.options.for_global_scope() streaming_handlers = global_options.streaming_workunits_handlers report_interval = global_options.streaming_workunits_report_interval callbacks = Subsystem.get_streaming_workunit_callbacks(streaming_handlers) streaming_reporter = StreamingWorkunitHandler( - self._scheduler_session, callbacks=callbacks, report_interval_seconds=report_interval + self.scheduler_session, callbacks=callbacks, report_interval_seconds=report_interval ) help_output = self._maybe_handle_help() if help_output is not None: self._exiter.exit(help_output) + v1 = global_options.v1 + v2 = global_options.v2 with streaming_reporter.session(): try: - engine_result = self._maybe_run_v2() - goal_runner_result = self._maybe_run_v1() + engine_result = self._maybe_run_v2(v2) + goal_runner_result = self._maybe_run_v1(v1) finally: run_tracker_result = self._finish_run() final_exit_code = self._compute_final_exit_code( diff --git a/src/python/pants/bin/remote_pants_runner.py b/src/python/pants/bin/remote_pants_runner.py index a674cabc3e3..757f77b5a69 100644 --- a/src/python/pants/bin/remote_pants_runner.py +++ b/src/python/pants/bin/remote_pants_runner.py @@ -149,7 +149,7 @@ def _run_pants_with_retry(self, pantsd_handle: PantsDaemon.Handle, retries: int traceback = sys.exc_info()[2] raise self._extract_remote_exception(pantsd_handle.pid, e).with_traceback(traceback) - def _connect_and_execute(self, pantsd_handle): + def _connect_and_execute(self, pantsd_handle: PantsDaemon.Handle): port = pantsd_handle.port # Merge the nailgun TTY capability environment variables with the passed environment dict. ng_env = NailgunProtocol.isatty_to_env(self._stdin, self._stdout, self._stderr) diff --git a/src/python/pants/engine/legacy_engine.py b/src/python/pants/engine/legacy_engine.py index 4f97a635818..0383ab47603 100644 --- a/src/python/pants/engine/legacy_engine.py +++ b/src/python/pants/engine/legacy_engine.py @@ -9,13 +9,13 @@ class Engine(ABC): """An engine for running a pants command line.""" - def execute(self, context, goals): + def execute(self, context, goals) -> int: """Executes the supplied goals and their dependencies against the given context. :param context: The pants run context. :param list goals: A list of ``Goal`` objects representing the command line goals explicitly requested. - :returns int: An exit code of 0 upon success and non-zero otherwise. + :returns an exit code of 0 upon success and non-zero otherwise. """ try: self.attempt(context, goals) diff --git a/src/python/pants/engine/scheduler.py b/src/python/pants/engine/scheduler.py index e48609b9b0b..424725e3d19 100644 --- a/src/python/pants/engine/scheduler.py +++ b/src/python/pants/engine/scheduler.py @@ -9,7 +9,7 @@ import traceback from dataclasses import dataclass from textwrap import dedent -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Type +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Type, cast from pants.base.exception_sink import ExceptionSink from pants.base.exiter import PANTS_FAILED_EXIT_CODE @@ -493,7 +493,7 @@ def _trace_on_error(self, unique_exceptions, request): unique_exceptions, ) - def run_goal_rule(self, product, subject): + def run_goal_rule(self, product, subject) -> int: """ :param product: A Goal subtype. :param subject: subject for the request. @@ -508,7 +508,7 @@ def run_goal_rule(self, product, subject): self._trace_on_error([exc], request) return PANTS_FAILED_EXIT_CODE _, state = returns[0] - return state.value.exit_code + return cast(int, state.value.exit_code) def product_request(self, product, subjects): """Executes a request for a single product for some subjects, and returns the products. diff --git a/src/python/pants/init/engine_initializer.py b/src/python/pants/init/engine_initializer.py index 3a33c2ede50..2b65751bde3 100644 --- a/src/python/pants/init/engine_initializer.py +++ b/src/python/pants/init/engine_initializer.py @@ -215,7 +215,7 @@ def run_goal_rules( options: Options, goals: Iterable[str], specs: Specs, - ): + ) -> int: """Runs @goal_rules sequentially and interactively by requesting their implicit Goal products.