How do I `yield Get` for a singleton?
# development
h
How do I
yield Get
for a singleton?
a
one way to work around it is to just use it as one of the params in the rule itself
h
Yeah, I thought of that, but it seems weird to do so
a
idk if there’s a way to request a singleton with yield, there probably should be
agreed
h
because this is an implementation detail
a
i agree
h
I may not need the SymbolTable tomorrow, so it seems weird to change the signature of the rule
a
oh
i think you might be able to do something like
h
it's not the end of the world
a
yield Get(MySingleton, Params())
let me get to my keyboard
this should be simple
h
Hmm, I don't understand the error I got for that
a
yes, sorry. i'm paging back in
h
I got a "Was not usable by any other @rule."
error
a
ok, so i think that
Params()
only works for
self.context._scheduler.product_request()
right now basically, the
Get()
code doesn't handle it at all
i think i remember that there was a conversation about exactly this a while ago and it may have led to a ticket, searching for that for a bit
aha! here is at least some more context: https://github.com/pantsbuild/pants/issues/7490
ok, so i think the answer right now is "we can't do singletons or multiple params in a
Get
right now, but we might be able to implement that without too much trouble" (as per https://github.com/pantsbuild/pants/issues/7490)
(but that doesn't need to be the canonical answer, just prior art)
h
OK, so "multiple params" also means "no params" 🙂
Would be nice to fix in the future, but at least there's an obvious workaround
a
yes! that seemed like a really nice unification to me
yes
the ticket describes what code you'd need to touch
grrrr on my todo list
a
@happy-kitchen-89482 There’s also a thing where IIRC you should definitely prefer signature injection over `yield Get`ing for performance reasons, and also changing the signature shouldn’t be problematic because only the engine should ever actually be calling the function?
w
you don't
yield Get
for a singleton: you put it in your argument list
(danny's response)
and yea, for performance reasons, like daniel said.
if you're not doing a projection, receiving the argument avoids starting running the rule until everything is available
a
...currently
w
well, there would never (?) be a good reason to suspend to request something that took no params... so even in the future it's probably the right answer
a
it's the right answer on the backend
https://github.com/pantsbuild/pants/issues/7490#issuecomment-479626879 describes how to rewrite rules to do that even if the
yield
is in the middle of the rule
well, that's what i'm thinking at least
the reason we'd want to maybe push `yield`s of singletons and of other things that are already satisfied by previously-requested params into the rule's top-level `Get`s is that it then becomes harder to write non-performant rules by default
but for now we can do it manually
w
i think that even if you "did it automatically", my preference would be for it to be a lint
👍 1
but yea. lots of other stuff to look at
a
agreed on it being a lint
this is the problem with autovectorization
it's way too magical
👍 1
lints are really nice to nip that in the bud
h
Makes sense, thanks!