It’s non-obvious how I should be canonically getting the “type” of a

HydratedTarget

. Mumble mumble https://github.com/pantsbuild/pants/issues/4535 mumble mumble, but right now, I can either check the type of

HydratedTarget.adaptor

(really weird abstraction leak), or I can check the string value of

HydratedTarget.adaptor.type_alias

, except that’s non-canonical (maybe

PythonTest

, maybe

python_test

, maybe another alias someone has installed!)

yea... handwaving there. but now is a good time to be thinking about what fits the model.

As in, I have n targets, give me a

PyTestResults

if you can give it to me, but skip any targets you can’t give me a thing for?

right. the @console_rule would be requesting

OptionalTestResult

, perhaps

That seems scary

i’m wary of introducing Optional as opposed to e.g. an enum type

Optional is an enum type 😃

Optional is a fixed enum type that is easily overloaded for a variety of use cases. i had interpreted the above to mean there would be machinery for understanding Optional, but perhaps not an enum type I define. I think the concern is too tied to an impl in my head to worry about now

no magic around Optional... it would be like Collection

i’ll check that out

Maybe not scary… But it confuses the model a bit… I will keep pondering…

the dep inference patch contains optional: https://github.com/twitter/pants/commit/d0780fb5dea940ef36c9c2d3dd8dfe17d49355e4#diff-ebd006be71f71c8ce84bba83f031fdc1

i mean you’re using Optional and exceptions, which is fine, but also doesn’t give the opportunity for another rule to recover from a failure matching a target for a jvm package name

So I think the place it gets scary is when you’re aggregating different types. If my rule needs an

OptionalTestResult

at the top level, which comes from assuming that all targets either produced an

OptionalPythonTestResult

or an `OptionalJunitTestResult`… I guess I’d want some higher-order

Map

and

Merge

kind of rules to make that work; but I don’t really want to build a generic higher order function system into the graph… And if some targets were known not to produce any test result (say binaries listed as root targets, when I’m running test)… Basically, before even thinking about this, I’d want to address https://github.com/pantsbuild/pants/issues/4535 because I think all of the places I’d currently want optional are horribly muddied because how I’d actually want to solve them is per-target-type Target types.

per-target-type Target types.

yea. my feeling is that 4535 allows for that

i feel like the dependency resolution we have with declarative selectors is really powerful and i still don't know why we need an enum type or a switch or any type of casting or any canonical monadic types like Optional (which simulate the logic we already have in the graph) instead of a rule subgraph with intermediate products. then instead of having to worry about how to structure optionals that maybe contain extra info and merging them we can just make sure your rule (which does whatever testing it wants) produces a collection of

class CompletedTestResult(datatype(...))

, or maybe a wrapper

class JunitCompletedTestResult(datatype([('test_result', CompletedTestResult)]))

around that collection which has a rule subgraph to convert it into a

CompletedTestResult

so the polymorphism is done by the logic the graph already contains. the only reason this isn't immediately doable (it seems) is because trivial rule subgraphs e.g. accessing a field still invoke some overhead, so we can't just willy-nilly create these simple synchronous rule subgraphs e.g. to extract the

test_result

field above (or so john was saying at one point in a PR i made?), but that's a fixable problem.

did you see my response on 4535

now i did

i am not suggesting subtyping.

and non-reliance on special types isn't a feature of the framework it's a feature of the ruleset

your response does not address the extensibility bit: concretely, how are you suggesting that we implement extensibility of a series of

if

statements

i don't understand how it doesn't address that. the rule graph is extensible

if there is already a rule to satisfy

X for Y

, installing another rule to do that will cause the graph to fail to build with an ambiguity error

yep

it will do that statically

that's correct

(it used to do it at runtime, sortof, somewhat. but the new/old rules are the same)

the framing of the problem seems incredibly narrow

you are.

yes

so, concretely: if there is a pytest runner rule installed, you may not install a junit runner

what is this pytest runner rule selecting

and, a switch statement that replaces some existing built in rule

@rule(TestResult, [Select(PytestConfig)])

and

@rule(TestResult, [Select(JunitConfig)])

it needs to request something abstract, which then needs to be filled in by multiple implementations

i've got plenty of code lol

@rule(TestResult, [Select(PytestConfig)])

and

@rule(TestResult, [Select(JunitConfig)])

the issue with this is that you still need the switch statement somewhere to create those configs

it's turtles all the way down actually

as i said: there is nothing to provide

JunitConfig

why wouldn’t that be provided by the junit ruleset?

how would something know that it could provide

JunitConfig

for some address?

because there’s a unique path in the rule graph from something -> JunitConfig

and to get back to the title of the ticket: this all started because we have a concrete case of this: you don't know till runtime whether a target will be an X or a Y

i can write that code too

the whole point of the last N comments on the ticket is that we do not have a facility to do that for the rule writer

if we have a rule from BuildFileAddresses -> Collection.of(JunitCompatibleTargetWrapper) we can ensure the switching is kept in the process of target selection instead of introducing optional into test results