hundreds-father-404
05/21/2020, 8:20 PMI still suggest that means we need to work out how to improve our syntaxGenerally agreed with this sentiment. But we’re also running up against natural constraints of the engine that we are unlikely to want to change. For example, needing a new request type and a result type will likely always be the case, afaict. The engine is exceptionally magical, in a good way. But sometimes plain and simple Python is more appropriate. —
We should work out how to do that kind of transition in the future without breaking the worldAgreed
witty-crayon-22786
05/21/2020, 10:07 PMattrs
? https://docs.bazel.build/versions/2.0.0/skylark/rules.html#providersGet
s would be helpful, we should think about it. it would allow you to uniformly do await target.field
, regardless of whether the field was async or nothundreds-father-404
05/21/2020, 10:20 PMwitty-crayon-22786
05/21/2020, 10:32 PMawait target.field
thing?hundreds-father-404
05/21/2020, 10:33 PMwitty-crayon-22786
05/21/2020, 10:34 PMawait target.field
, yea. i agree.fields = await (target.sources, target.dependencies)
average-vr-56795
05/21/2020, 10:37 PMjoin_all
combinator along the lines of:
await [target.field, target.other_field]
witty-crayon-22786
05/21/2020, 10:37 PMaverage-vr-56795
05/21/2020, 10:37 PMwitty-crayon-22786
05/21/2020, 10:37 PMhundreds-father-404
05/21/2020, 10:38 PMawait target[Field]
means the call site is acceptable, but we need to also keep boilerplate minimal for plugin authors
Also, nb that HydrateSources
takes parameters, like enable_codegen
. await tgt[Sources]
would lose that ability, unless we had some special casewitty-crayon-22786
05/21/2020, 10:38 PMGet[HydratedSources](HydrateSourcesRequest, ..)
Get
… await target.sources(PythonSources)
hundreds-father-404
05/21/2020, 10:40 PM.request
property. See https://github.com/pantsbuild/pants/pull/9632witty-crayon-22786
05/21/2020, 10:41 PMall_hydrated_sources = await MultiGet(
1. Get[HydratedSources](HydrateSourcesRequest, tgt.get(Sources).request) for tgt in targets
2. Get[HydratedSources](HydrateSourcesRequest(tgt.get(Sources))) for tgt in targets
3. tgt.get(Sources)
)
hundreds-father-404
05/21/2020, 10:42 PM@property
on every AsyncField
that generates the request for you, becaause @property
doesn’t allow parameters. You could do it for most AsyncFields
, but not all, and it’s not obvious when you can vs. cannotwitty-crayon-22786
05/21/2020, 10:43 PMGet
)average-vr-56795
05/23/2020, 8:31 AMwitty-crayon-22786
05/26/2020, 5:26 PMenough-analyst-54434
05/29/2020, 4:31 PMtest_addresses = Addresses((field_set.address,))
transitive_targets = await Get[TransitiveTargets](Addresses, test_addresses)
After:
async def setup_pytest_for_target(
...,
get_transitive_targets: GetRule[TransitiveTargets, Addresses]
) -> TestTargetSetup:
test_addresses = Addresses((field_set.address,))
transitive_targets = await get_transitive_targets(test_addresses)
The idea is probably self explanatory given signatures, but GetRule[X, Y] produces a unique singleton type for the pair X, Y and associated singleton rule producing a singleton instance of that type. GetRule is a callable that takes a subject and produces a Get(X, Y, subject).
Nay, Hrm or Yay?hundreds-father-404
05/29/2020, 4:35 PMget_transitive_targets: GetRule[TransitiveTargets, Params[Addresses, Foo]]
enough-analyst-54434
05/29/2020, 4:36 PMhundreds-father-404
05/29/2020, 4:37 PMawait get_transitive_targets(s1, s2, s3)
MultiGet
look like?enough-analyst-54434
05/29/2020, 4:39 PMMultiGet(get_rule1(...), get_rule2(...))
Or:
MultiGet(get_rule(x) for x in xs)
async def ...
) and rule parameters. You can ask the engine to find one of these rules for you with a GetRule parameter; ie: link to other peoples rules, just not by name, which is the point of the whole plugin / dependency injection system.hundreds-father-404
05/29/2020, 4:48 PMlist_targets.py
using either Get[Targets]
or Get[TransitiveTargets]
. I like that that is encapsulated in the call site and doesn’t leak. I think I like this because I view it as reducing the surface area of what I need to consider.
But, rules are not normal. Here, the coupling might not be a bad thing. There’s an argument that it centralizes all the magic of the engine into one place. So, I’m trying to keep an open mind.enough-analyst-54434
05/29/2020, 4:49 PMhundreds-father-404
05/29/2020, 4:50 PMenough-analyst-54434
05/29/2020, 4:51 PMwitty-crayon-22786
05/29/2020, 4:51 PMenough-analyst-54434
05/29/2020, 4:51 PMhundreds-father-404
05/29/2020, 4:52 PMYou aren’t really, you don’t express the type parameters more than once.Yes, but this no longer gets expressed in the same location as the call site. “Principle of Proximity” is more apt than coupling for my concern.
enough-analyst-54434
05/29/2020, 4:52 PMhat probably means the concept isn’t actually removed?It is - Get would be a hidden type
witty-crayon-22786
05/29/2020, 4:52 PMenough-analyst-54434
05/29/2020, 4:52 PMwitty-crayon-22786
05/29/2020, 4:54 PMenough-analyst-54434
05/29/2020, 4:55 PMwitty-crayon-22786
05/29/2020, 4:57 PMenough-analyst-54434
05/29/2020, 4:57 PMwitty-crayon-22786
05/29/2020, 4:59 PMawait
== suspensionenough-analyst-54434
05/29/2020, 4:59 PMawait Get
- if that's broken already that's another problem.witty-crayon-22786
05/29/2020, 5:00 PMhundreds-father-404
05/29/2020, 5:00 PMpytest_runner.py
would be a great example.enough-analyst-54434
05/29/2020, 5:00 PMwitty-crayon-22786
05/29/2020, 5:01 PMenough-analyst-54434
05/29/2020, 5:02 PMunderstood. i’m clarifying what i was saying inK - I think though, since the switch to await did not change Python <-> rust concurrency dynamics or code at all, that the old point was wrong in the large and maybe right about me using poor language. IE we could have always done this.
witty-crayon-22786
05/29/2020, 5:02 PMthe first time this was proposed, it was as callables that actually return the value^ this is the thing that runs afoul of suspension… that’s all i’m saying
enough-analyst-54434
05/29/2020, 5:03 PMwitty-crayon-22786
05/29/2020, 5:03 PMenough-analyst-54434
05/29/2020, 5:03 PMwitty-crayon-22786
05/29/2020, 5:03 PMenough-analyst-54434
05/29/2020, 5:04 PMwitty-crayon-22786
05/29/2020, 5:05 PMhundreds-father-404
05/29/2020, 5:06 PMawait Get[Targets](Addresses([..]))
vs.
await get_targets(Addresses([..])
witty-crayon-22786
05/29/2020, 5:06 PMenough-analyst-54434
05/29/2020, 5:06 PMwitty-crayon-22786
05/29/2020, 5:07 PMawait get_targets(addresses)
… no need for the type declaration to be on the same line anymorehundreds-father-404
05/29/2020, 5:08 PMget_targets()
will give me back Targets
.
Arguably, the parameter name makes that obvious. But the call site now de-emphasizes types. I like the emphasis on types because this is a type-driven APIenough-analyst-54434
05/29/2020, 5:11 PMwitty-crayon-22786
05/29/2020, 5:12 PMhundreds-father-404
05/29/2020, 5:13 PMenough-analyst-54434
05/29/2020, 5:15 PMwitty-crayon-22786
05/29/2020, 5:17 PMaverage-vr-56795
05/29/2020, 6:48 PMWith the change, I need to refer back to my rule signature to know thatWhy should this be different from the rest of the language?will give me backget_targets()
.Targets
hundreds-father-404
05/29/2020, 7:12 PMWhy should this be different from the rest of the language?Because the rest of the language doesn’t place a huge weight on types, even with MyPy. In contrast, you must use types for the engine. And you must also use them in a particular way, e.g. not being able to request a subclass of a registered type
witty-crayon-22786
06/08/2020, 8:30 PM