https://pantsbuild.org/ logo
Join Slack
Powered by
I wonder if the right thing to do here is introduc...
# development
h
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
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
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
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
yeah I think keeping the scope of the option limited to 
PyTest
makes sense
w
it feels like a general “all test targets” facility, rather than being pytest specific.
h
True. For example, if someone adds Hypothesis, they presumably want the same env setting to apply there
w
which might mean that it is a 
test
goal option, and a generic field type for “testable” field sets
h
that would work too, and I guess that way the 
run_tests
goal rule could be in charge of parsing it
h
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
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
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
there's nothing wrong with a goal rule just invoking 
os.environs
, right?
w
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
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
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
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
Open in Slack
PreviousNext
Powered by