Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify subsystem registration. #12620

Merged
merged 1 commit into from
Aug 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions src/python/pants/build_graph/build_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from __future__ import annotations

import logging
import typing
from collections import defaultdict
from collections.abc import Iterable
from dataclasses import dataclass, field
Expand Down Expand Up @@ -137,9 +136,6 @@ def _register_exposed_object(self, alias, obj):
)

self._exposed_object_by_alias[alias] = obj
# obj doesn't implement any common base class, so we have to test for this attr.
if hasattr(obj, "subsystems"):
self.register_subsystems(obj.subsystems())

def _register_exposed_context_aware_object_factory(
self, alias, context_aware_object_factory
Expand All @@ -154,13 +150,7 @@ def _register_exposed_context_aware_object_factory(
alias
] = context_aware_object_factory

# NB: We expect the parameter to be Iterable[Type[Subsystem]], but we can't be confident
# in this because we pass whatever people put in their `register.py`s to this function;
# I.e., this is an impure function that reads from the outside world. So, we use the type
# hint `Any` and perform runtime type checking.
# TODO: Is this still true? Do we still support a hook for registering subsystems,
# instead of just having them be discovered as rule params?
def register_subsystems(self, subsystems: typing.Iterable[Type[Subsystem]] | Any):
def register_subsystems(self, subsystems: Iterable[Type[Subsystem]]):
"""Registers the given subsystem types."""
if not isinstance(subsystems, Iterable):
raise TypeError("The subsystems must be an iterable, given {}".format(subsystems))
Expand Down Expand Up @@ -202,7 +192,7 @@ def register_rules(self, rules):
# this because we pass whatever people put in their `register.py`s to this function;
# I.e., this is an impure function that reads from the outside world. So, we use the type
# hint `Any` and perform runtime type checking.
def register_target_types(self, target_types: typing.Iterable[Type[Target]] | Any) -> None:
def register_target_types(self, target_types: Iterable[Type[Target]] | Any) -> None:
"""Registers the given target types."""
if not isinstance(target_types, Iterable):
raise TypeError(
Expand Down
7 changes: 1 addition & 6 deletions tests/python/pants_test/init/test_extension_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,7 @@ class DummyObject1:


class DummyObject2:
# Objects can have a special classmethod `subsystems` to register subsystems.
@classmethod
def subsystems(cls):
return (DummySubsystem,)
pass


@dataclass(frozen=True)
Expand Down Expand Up @@ -158,7 +155,6 @@ def test_load_valid_partial_aliases(self):
registered_aliases = build_configuration.registered_aliases
self.assertEqual(DummyObject1, registered_aliases.objects["obj1"])
self.assertEqual(DummyObject2, registered_aliases.objects["obj2"])
self.assertEqual(build_configuration.subsystems, FrozenOrderedSet([DummySubsystem]))

def test_load_invalid_entrypoint(self):
def build_file_aliases(bad_arg):
Expand Down Expand Up @@ -280,7 +276,6 @@ def reg_alias():
registered_aliases = build_configuration.registered_aliases
self.assertEqual(DummyObject1, registered_aliases.objects["FROMPLUGIN1"])
self.assertEqual(DummyObject2, registered_aliases.objects["FROMPLUGIN2"])
self.assertEqual(build_configuration.subsystems, FrozenOrderedSet([DummySubsystem]))

def test_rules(self):
def backend_rules():
Expand Down