https://pantsbuild.org/ logo
#development
Title
# development
h

hundreds-father-404

01/31/2020, 2:15 AM
Any ideas if it’s possible to synchronously request a subsystem registered with
subsystem_rule
? These both failed
Copy code
changed_subsystem, = session.product_request(Changed, [Options, options])  # Options is not a RootRule
Copy code
changed_subsystem, = session.product_request(Changed, [])  # Must give a subject
a

aloof-angle-91616

01/31/2020, 2:16 AM
i think you can always make something a RootRule too by just adding that to the list of rules
but the problem you describe would be great to address
h

hundreds-father-404

01/31/2020, 2:17 AM
Agreed, but I think we likely don’t want to make
Options
be a
RootRule
only to support this edge case I’m wondering how the async API ends up getting the Subsystem when it’s in a rule signature? I think that we convert the Subsystem into a TaskRule?
a

aloof-angle-91616

01/31/2020, 2:18 AM
Is there an equivalent problem with
await Get[X](...)
? i suppose we still don’t support params in Get yet
h

hundreds-father-404

01/31/2020, 2:18 AM
hm, I haven’t tried that
we always request subsystems in the rule signature
a

aloof-angle-91616

01/31/2020, 2:19 AM
Agreed, but I think we likely don’t want to make
Options
be a
RootRule
only to support this edge case
i think this might actually be ok if necessary!
I’m wondering how the async API ends up getting the Subsystem when it’s in a rule signature? I think that we convert the Subsystem into a TaskRule?
what do you mean “in a rule signature”? do you mean “not a Get”?
h

hundreds-father-404

01/31/2020, 2:20 AM
Copy code
@rule
def my_rule(isort: Isort) -> LintResult
Rather than
Copy code
@rule
def my_rule() -> LintResult:
  isort = await Get[Isort](..)
a

aloof-angle-91616

01/31/2020, 2:20 AM
if you check optionable.py you’ll see where we convert to a TaskRule
it’s a method ending in
_signature
h

hundreds-father-404

01/31/2020, 2:21 AM
Copy code
@classmethod
  def signature(cls):
    """Returns kwargs to construct a `TaskRule` that will construct the target Optionable.

    TODO: This indirection avoids a cycle between this module and the `rules` module.
    """
    partial_construct_optionable = functools.partial(_construct_optionable, cls)

    # NB: We must populate several dunder methods on the partial function because partial functions
    # do not have these defined by default and the engine uses these values to visualize functions
    # in error messages and the rule graph.
    snake_scope = cls.options_scope.replace('-', '_')
    partial_construct_optionable.__name__ = f'construct_scope_{snake_scope}'
    partial_construct_optionable.__module__ = cls.__module__
    _, class_definition_lineno = inspect.getsourcelines(cls)
    partial_construct_optionable.__line_number__ = class_definition_lineno

    return dict(
        output_type=cls.optionable_cls,
        input_selectors=tuple(),
        func=partial_construct_optionable,
        input_gets=(Get.create_statically_for_rule_graph(ScopedOptions, Scope),),
        dependency_optionables=(cls.optionable_cls,),
      )
a

aloof-angle-91616

01/31/2020, 2:21 AM
that is then consumed in rules.py
h

hundreds-father-404

01/31/2020, 2:21 AM
I think
input_gets=(Get.create_statically_for_rule_graph(ScopedOptions, Scope),)
is relevant? Does this suggest that you should pass
ScopedOptions
?
a

aloof-angle-91616

01/31/2020, 2:22 AM
ScopedOptions is almost definitely going to be provided for you i think, i’d try ripgrep searching that symbol
h

hundreds-father-404

01/31/2020, 2:23 AM
changed_subsystem, = session.product_request(Changed, [Scope(Changed.options_scope)])
leads to
Copy code
Exception message: No installed @rules can compute Changed for input Params(Scope), but there were @rules that could compute it using:
  Params(OptionsBootstrapper)
a

aloof-angle-91616

01/31/2020, 2:23 AM
where is this erroring?
is this in a test?
h

hundreds-father-404

01/31/2020, 2:24 AM
When running
./pants
a

aloof-angle-91616

01/31/2020, 2:24 AM
oh
oh well
h

hundreds-father-404

01/31/2020, 2:24 AM
(I’m trying to add
changed_subsystem, = session.product_request(Changed, [Scope(Changed.options_scope)])
to
specs_calculator.py
. It has access to
Options
, but not
OptionsBootstrapper
)
a

aloof-angle-91616

01/31/2020, 2:25 AM
i also have a PR that rewrites changed calculation as prework for --query
it uses @rules and doesn’t have this issue because it avoids using the sync API
what branch are you on?
h

hundreds-father-404

01/31/2020, 2:25 AM
I saw! But it looked like that was blocked so I’m trying to cherry-pick the parts that we can land as prework for deprecating
--owner-of
a

aloof-angle-91616

01/31/2020, 2:26 AM
yes!
i was going to attempt to determine whether that PR (or some of it) could work around the issue you’re seeing
h

hundreds-father-404

01/31/2020, 2:26 AM
Here’s the WIP: https://github.com/pantsbuild/pants/compare/master...Eric-Arellano:decouple-changed?expand=1 My main goal is “decouple
--owned
and
--changed-*
. This is an optimization to remove the
ChangedRequest
dataclass - not critical, only would be neat to do
a

aloof-angle-91616

01/31/2020, 2:26 AM
i’m still not quite sure why those errors are happening atm
h

hundreds-father-404

01/31/2020, 2:27 AM
The second commit does everything I want, I only would like to land the 3rd if possible to make things cleaner
a

aloof-angle-91616

01/31/2020, 2:27 AM
why are we removing the ChangedRequest dataclass?
h

hundreds-father-404

01/31/2020, 2:28 AM
It’s a weird wrapper around the
Changed
subsystem that duplicates it’s functionality. Those methods should live in the subsystem, in a perfect world. We only have
ChangedRequest
as a way to consume the subsystem outside of V2 rules
a

aloof-angle-91616

01/31/2020, 2:28 AM
--query makes use of it effectively
if this seems better to you, go for it
h

hundreds-father-404

01/31/2020, 2:29 AM
Could
--query
directly request the
Changed
subsystem? That’s how all other rules work. This is the only instance where we wrap the options defined in a
Subsystem
a

aloof-angle-91616

01/31/2020, 2:31 AM
? not really? i’ve started to use the Factory inner class subsystem pattern a lot to get a dataclass from subsystem options, and i then expose it with a single @rule which converts from the subsystem to a dataclass
i like that this approach decouples “the data a subsystem provides” from “the literal class declaration of a specific subsystem with a specific options_scope”
h

hundreds-father-404

01/31/2020, 2:32 AM
Hm okay I’m going to give up on this part of the PR - not very important to the filesystem specs changes I’m making
👍 1
a

aloof-angle-91616

01/31/2020, 2:32 AM
i was also going to say that but didn’t wanna slow you down
❤️ 1
plz lmk if you want to pair on any followup
h

hundreds-father-404

01/31/2020, 2:33 AM
Thanks for talking it over! Will have this up for review in a couple minutes after some cleanup and adding a description 🙂
a

aloof-angle-91616

01/31/2020, 2:33 AM
thanks for the work!!!!
❤️ 1
👖 1
w

witty-crayon-22786

01/31/2020, 5:01 PM
check the visualization
to see what is needed to compute Changed
we should probably have a CLI-rendered visualization as well for this case.
👍 1