https://pantsbuild.org/ logo
Join Slack
Powered by
Following up on a discussion of the environments r...
# development
a
Following up on a discussion of the environments redesign I just had with @hundreds-father-404 regarding how this will affect Pants’ ease of adoption while we add in support for more advanced use cases. The tl;dr is that we think of environments as a new way of setting (applicable) subsystem options, rather than replacing them outright. Further commentary in-thread…
💯 1
➕ 1
We’ve noticed a disconnect between out-of the-box pants configuration, and what are currently easy configuration tasks (e.g. setting global search paths for Python), which the current environments proposal makes significantly more difficult than the status quo. After some brainstorming, we realised that it should be possible to incrementally adopt environment-specific features, and then easily switch to multi-environment support when you discover that you need it. My proposal is to treat environments as an “override” mechanism for selected options, by introducing a concept of “singleton options” (i..e our current options behaviour) and “environment-specific options” (new behaviour). Only “environment-specific options” will be allowed to be set in environment declarations. These will be opt-in at the options API level. This would allow users to have a global configuration for these options at first, and then when you move to needing environments, you can migrate the configurable options incrementally as you need them, rather than switching to the environments idea entirely. There’s a few problems I haven’t fleshed out here, particularly in terms of how we model the configuration. But I think this will make it significantly easier for plugin authors to add support for consuming environments (or to choose not to), and to make it easier to treat environments as an advanced use case, rather than a necessary early cliff in Pants’ learning curve.
I’m going to let this idea stew over the weekend and have a go at implementing this next week barring major objections
h
I overall am strong +1 for keeping the current options like 
[python-bootstrap].search_path
around, and it providing the default value for all environments (including no envs defined). That makes things much simpler when environments can be 
n = 1
Environment targets now only override the options system
💯 1
a
One more note is that the remainder of the modelling of environments — as targets that fit into the remainder of the target graph — per the design doc is very sound, and fits into this idea. It’s just what those targets end up modelling that changes slightly.
👍 1
h
For the technical implementation, I continue to suggest an alternative one then it sounds like you want to pursue: 1. For the environment fields, get rid of 
default
and simplify our 
help
message to be nothing more than `"Overrides 
[python-bootstrap].binary_names
."` 2. Nothing needs to change about the options system, other than 
help
message to point out the relevant field 3. At our consumption sites, you will call something like
Copy code
env_tgt.field_or_option(PythonBootstrapBinaryNamesField, python_bootstrap_subsystem, "search_path")
I caution that instead consuming the value via the 
Subsystem
instance may be an uphill battle. It can't use the Rules API, so you can't get magical — you must pass the 
env_tgt: ResolvedEnvironmentTarget
to the subsystem. So, it would now be something like 
python_bootstrap_subsystem.search_path(env_tgt)
, which makes accessing options much more complex for things that don't need it
a
(note for Tuesday: I suspect it would be possible for singleton options to not need to pass in an 
env_tgt
, but that’s a thing I want to explore further.)
w
if the end goal is still for options to live on Subsystems, then you could change the creation of the Subsystem to be environment specific.
i.e., adjust https://github.com/pantsbuild/pants/blob/d2b22d1594803c6a13dfcf6915a02e3b770f91dc/src/python/pants/option/subsystem.py#L43-L77 to consume the environment, and create an instance with the relevant options overridden
currently, creating a subsystem boils down to this 
@rule
, partially applied with the subsystem to create: https://github.com/pantsbuild/pants/blob/d2b22d1594803c6a13dfcf6915a02e3b770f91dc/src/python/pants/option/subsystem.py#L133-L135 … if you adjusted that 
def signature
/ and 
@rule
to take an Environment, you could construct the subsystem with the overrides
(sorry i missed this earlier)
if you adjusted that 
def signature
/ and 
@rule
to take an Environment, you could construct the subsystem with the overrides
…after https://github.com/pantsbuild/pants/pull/16721 lands
h
the end goal is still for options to live on Subsystems, then you could change the creation of the Subsystem to be environment specific.
the idea is: • still keep the subsystems for when you're fine with exactly 1 env (and possible overrides via pants.rc) • otherwise, users define evironments using the new code from this week. subsystems give the defaults so I don't think we need to adjust how the subsystem is created?
w
i’m thinking about it the opposite way
if the options are still going to be defined on the Subsystem, then why not construct the Subsystem using the appropriate (overridden if need be) values in the first place
i.e. “consume environments and options to construct a Subsystem and then consume a Subsystem” rather than: “consume a Subsystem and an environment”
h
Ahhhh, I see. So that consumers still only need to access 
python_bootstrap_subsystem.search_path
and It Just Does The Right Thing
w
exactly
h
That's exactly what Chris is after 🙂 I like it
w
i do think that if the options values are going to continue to live on the Subsystems rather than migrating though, it’s worth re-examining assumptions in the design based on having ruled them out
a
@witty-crayon-22786 This is exactly why I left the thing to stew for the weekend rather than diving straight in. I think getting everything defined in the subsystem means we won’t need to do any of the options migration work, it’s just a matter of definining which options are environment specific
and it makes this a more incremental change. I Agree about revisiting the design doc. I think on Tuesday I write a two-pager about how I think this should work, and we update the rest of the design based on that?
h
I think getting everything defined in the subsystem means we won’t need to do any of the options migration work, it’s just a matter of definining which options are environment specific
Depending on how we have the fields defined in the local environment target. I continue to personally bias towards this:
Copy code
local_environment(
  python_bootstrap_search_paths=["<PYENV>"],
  golang_executable_search_paths=["<PATH>"],
)
over this
Copy code
local_environment(
   options={
      "[python-boostrap].search_paths": ["<PYENV>"],
     "[golang].executable_search_paths": ["<PATH>"],
    },
)
It makes 
./pants help
nicer, and we eagerly get things like data validation & rejection of unrecognized fields 
help
would look like:
Copy code
Overrides [python-bootstrap].binary_names.
I like that 
./pants help local_environment
makes clear which options can be used and which cannot
w
I Agree about revisiting the design doc. I think on Tuesday I write a two-pager about how I think this should work, and we update the rest of the design based on that?
possibly… talk with @hundreds-father-404 on the mechanics of how to go about it probably.
h
possibly… talk with @hundreds-father-404 on the mechanics of how to go about it probably.
I don't think a ton will need to change. Targets still make sense for defining environments; consuming targets like 
python_test
still have 
environment: str
field the same as before But yeah, sounds good to re-evaluate on Tuesday
w
too late on a holiday Friday to dive in further, but yea: i’d suggest that you two sync next week.
☀️ 1
a
@hundreds-father-404 I want to be clear, I think the options dict is a terrible idea 😉
👍 1
(but I also don’t want to get tied up on using fields if we can’t automate it; writing options in config xor in fields is a sub-par author experience)
((it just so happens that fields do a lot of the validation magic for us, and I totally understand the bias to reusing stuff we already have))
h
but I also don’t want to get tied up on using fields if we can’t automate it
I expect you can DRY this pretty nicely. Field subclassing is heavily encouraged, so you can develop 
EnvironmentFieldMixin
that automates almost everything. And then Stu's suggestion can be combined with your options registration idea so that it's even better Including because this is opt-in, I'm not that worried about having an extra 3-4 lines of simple boilerplate for this new mechanism. It's also not even very common you need to define a new EnvironmentField
a
I’ve spent a bit dwelling on this, and I think it would be a mistake for the implementer to have to define environment-sensitive options as fields. I think all of the config-related code should live in the same place, so that there isn’t a conceptual gap between singleton options and environment-sensitive ones. To the implementer, they’re all options, and it’d probably be easy to introduce a backwards incompatibility as you move from an option to a field. That said, using the fields mechanism for validation of user input totally makes sense — the documentation and validation support is extremely user-friendly
I think we might be able to solve this by generating fields at runtime, by inspecting each option that is marked as environment-sensitive, so the 
local_environment
target always represents a subset of what is available through the options system. Advantages: • We can systematically generate the names of the fields, so that there is consistent namespacing of fields, and field names will always match the corresponding option names • Migrating an option from singleton to environment-sensitive is a one-line change • We still get all the data validation advantages of using BUILD files Disadvantages: • If implemented naïvely, there is a cycle between creating the fields and creating the subsystem (not sure this is insurmountable, but it gives me pause) • The origin of these fields is opaque to the casual observer
w
I think all of the config-related code should live in the same place, so that there isn’t a conceptual gap between singleton options and environment-sensitive ones. To the implementer, they’re all options, and it’d probably be easy to introduce a backwards incompatibility as you move from an option to a field.
in general i agree that this would be preferable. haven’t really looked at the mechanism though. will let you and Eric talk through this a bit before i revisit.
a
@witty-crayon-22786 The one thing that would get in the way here is whether I’d be able to inspect the options list of a subsystem in advance of constructing it. Is that possible?
w
yea, you can.
sec
Josh Cannon worked on that code most recently
a
I vaguely recall that. But yeah, with that method, It should be possible to generate fields for each option type on a subsystem
It sounds like the help output etc is unlikely to be upset by there being multiple instances of a substem, either?
w
well… no, it’s not a problem to have multiple instances of the Subsystem class created with different values.
👍 1
there is still one “name”/“scope” for the Subsystem… the 
help
for that Subsystem would still display the 
default
value, etc. the change is just that how things end up overridden by environments won’t be obvious in the help without some extra work there.
a
I am leaning towards it being a conflict for an option to be set in a singleton context AND an environment context
w
🤷
a
particularly because I think there isn’t an obvious “winner” if you set an option with a command line option and also an environment marker
w
i’m really unsure of the design at this point, because this is now much more options-heavy and less target-heavy, and so some of the tradeoffs from the design are less clear to me.
a
yup
I am trying to explore the parameters here
this should yield a design
@witty-crayon-22786 Here’s a first pass at a design, intended to capture the discussion from this thread: https://docs.google.com/document/d/14xYllPF_FjFMtJybTi2l_bZKfZsHvzGta7CIggdm_ms/edit?usp=sharing
w
thanks: let me know when you and Eric are on the same page about it and i’ll review
a
we are
👍 1
h
we are. but @witty-crayon-22786 is this gonna create a graph cycle? to consume subsystems, you need the EnvironmentName. To get the EnvironmentName, we need to consult subsystems (
[environments-preview]
at least)
(currently trying to figure out a graph cycle with consuming your code)
w
i think that the environments should likely be a global/bootstrap option
but… no, i don’t think that it necessarily creates an (impossible) graph cycle. it’s ok for rules to recurse
a
@witty-crayon-22786 Do we have a good grasp of which class of user is likely to be defining environments? I had assumed it would be developers rather than codebase admins, but maybe I’m wrong there. I think it matters from the point of view of where to define the environments
w
good question. my assumption so far had been that environments would be similar to resolves in that regard: mostly admins.
in the absence of something like pants, you might assume that every team has their own base image (to contain their specific thirdparty requirements/etc)… but with pants, the base image(s) / environments would be much fewer, and teams would then be building/deploying images built from the base images / environment
it’s sortof like OS images for physical machines… you wouldn’t want a proliferation of OS versions that need updating either
👍 1
a
Interesting — we’re definitely conceptualising Docker in a different way to how Docker is usually conceptualised
w
my assumption so far had been that environments would be similar to resolves in that regard: mostly admins.
and to be clear: this is “an admin blesses the creation of a new environment/resolve, but an end user might be editing it day to day”. that’s definitely the case for a resolve (all the requirements need edits on an ongoing basis), but whether it’s the case for an environment is unclear (changing env vars all the time…?)
👍 1
Interesting — we’re definitely conceptualising Docker in a different way to how Docker is usually conceptualised
yea. i think because this use case in particular is for the consumption of images, rather than their production.
a
Right, and I think most development teams think of the production of images as the way that they deploy their work, rather than consuming images that let them deploy their work
w
to produce an image users will use a 
docker_image
target… those we’re expecting lots of
a
and then those aren’t images that can be easily used as an environment, right?
w
eventually, yea. i don’t think we’re planning on that in a first version
➕ 1
a
I think the target-based modelling probably maps much better to being able to consume images for future build steps
h
great point, I didn't think about that
a
but that’s not a great reason for using targets Right Now™
it could be a good reason to stay the course and use targets
First pass on environment-sensitive options implementation is here: https://github.com/pantsbuild/pants/pull/16840 It’s definitely not complete or tested, but some feedback on the general approach would be appreciated
Open in Slack
PreviousNext
Powered by