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

hundreds-breakfast-49010

08/31/2020, 11:04 PM
I wonder if the right thing to do here is introduce a new python type
PantsEnvironment
(or similar name) that the test
goal_rule
will always generate from
os.environs
and add as an additional
Param
when it makes its own
Get
requests
w

witty-crayon-22786

08/31/2020, 11:19 PM
this sounds feasible, but note that you’ll want to filter it aggressively to only include stuff that might be relevant.
because each Param is part of the identity of all of the nodes/rules down to the one that actually consumes it, so you want to ensure that only relevant environment variables invalidate tests.
h

hundreds-breakfast-49010

08/31/2020, 11:25 PM
hm, yeah; and I don't know if we can know how to filter it aggressively without having access to this new option, which we're positing should live on the
PyTest
subsystem
we don't want to accidentally invalidate rules when some unrelated thing in pants' environment changes
maybe this options shouldn't actually live on
PyTest
after all?
h

hundreds-father-404

08/31/2020, 11:28 PM
Where else do you think it might live? The end effect is that we only want
pytest_runner.py
to use it when setting its
env
. No other processes like
black/rules.py
should use it
h

hundreds-breakfast-49010

08/31/2020, 11:29 PM
yeah I think keeping the scope of the option limited to
PyTest
makes sense
w

witty-crayon-22786

08/31/2020, 11:29 PM
it feels like a general “all test targets” facility, rather than being pytest specific.
h

hundreds-father-404

08/31/2020, 11:30 PM
True. For example, if someone adds Hypothesis, they presumably want the same env setting to apply there
w

witty-crayon-22786

08/31/2020, 11:30 PM
which might mean that it is a
test
goal option, and a generic field type for “testable” field sets
h

hundreds-breakfast-49010

08/31/2020, 11:30 PM
that would work too, and I guess that way the
run_tests
goal rule could be in charge of parsing it
h

hundreds-father-404

08/31/2020, 11:31 PM
and I guess that way the run_tests goal rule could be in charge of parsing it
Which means you don’t need
PantsEnvironment
, as the
@goal_rule
won’t cache reading the env var? Generally, I like
PantsEnvironment
. But also generally, until we have time for more proper design, we shouldn’t generalize more than necessary
h

hundreds-breakfast-49010

08/31/2020, 11:33 PM
yeah, I think it's not clear what
PantsEnvironment
should be absent some kind of global pants environment filtering functionality, unless it's literally just a copy of pants' environment. and if it's that literal copy we run into the problem with accidental cache invalidation when pants' environment changes spuriously
w

witty-crayon-22786

08/31/2020, 11:36 PM
an uncacheable intrinsic to read the environment seems like a good starting point though. then try to only use it “shallowly” in the graph to start (ie, try to only consume from a goal_rule for now)
👍 1
the way uncacheable nodes work, you could read from it deeper in the graph. i’m just not sure how much overhead it would add to do so
h

hundreds-breakfast-49010

08/31/2020, 11:37 PM
there's nothing wrong with a goal rule just invoking
os.environs
, right?
w

witty-crayon-22786

08/31/2020, 11:38 PM
just that you’d have to label it as unsafe, and add comments to indicate that it’s only safe to do in a goal_rule
h

hundreds-breakfast-49010

08/31/2020, 11:40 PM
it's easy enough to add that kind of comment, but we already have the expectation that you can do things in a
goal_rule
that you can't do in a normal
@rule
, right?
certainly we could make this explicit in the pants docs if it isn't already
I'm not sure what an uncacheable intrinsic gets us over just using
os.environs
, if we can only use itin a
goal_rule
anyway
w

witty-crayon-22786

08/31/2020, 11:41 PM
Types are better than comments.
💯 1
Not a blocker though
(also, adding a safe facility to do it doesn't prevent people from doing the unsafe thing, unfortunately)
h

hundreds-breakfast-49010

08/31/2020, 11:49 PM
yeah, adding more robust checks for what one can safely do in a rule is good
hm, does pants currently validate that only uncacheable rules can
Get
a
@side_effecting
type, or does that validation work for params only?
looks like the latter - we check this in python at rule definition time, and only look at input parameters
but maybe this is something an improved rule graph could do
👍 1