added `run in sandbox` field to `pex

Question

< happy kitchen 89482> added `run in sandbox` field to `pex binary` which currently hasn t been put in a stable release yet reverts that change in lieu of another change that offers the same behavior but in a different way We need to figure out our path forward for the current release 1 Leave the feature then do a full deprecation cycle and remove 2 Change the field to `experimental ` so we could pull the plug without deprecation 3 Get PR 15849 in I m only waiting on docs addition and an open TODO I want feedback on As the author of the PR my vote will obviously be 3 blush but I d like us to be intentional on our path forward

bitter-ability-32190 · Answer

CC < sparse lifeguard 95737> who cares very much for their org slightly smiling face

sparse-lifeguard-95737 · Answer

Copying from the other thread to be clear I don t really care either way about `run in sandbox` on `pex binary` vs un sandboxed `run` directly on a python script I can make either one work for my purposes I ve had people breathing down my neck about django management CLI + pants not working as expected and was going to point at the new `run in sandbox` field and say look it s going to work soon when I saw chatter here about possibly reverting the addition of that field in 2 13 I don t mind if it s reverted I just want to know if it s going to be reverted so I don t bait and switch my team

hundreds-father-404 · Answer

bump on this < happy kitchen 89482> I recommend we don t land the `run` change in 2 13 because we already are at a0 and have so many big changes but < bitter ability 32190> and < sparse lifeguard 95737> how do we feel about keeping `run in sandbox` but calling it `experimental run in sandbox` so that we can remove it more easily

sparse-lifeguard-95737 · Answer

experimental works for me +1

bitter-ability-32190 · Answer

Thats my option 2 slightly smiling face No qualms here Also FWIW if Dan can stomach the pain the escape hatch is an in repo plugin which has this functionality its what we re using at my work repo

bitter-ability-32190 · Answer

I think as long as we re ready to brace the inevitable question s of where did this thing go I was using it no harm in renaming

hundreds-father-404 · Answer

With `experimental run in sandbox` I still recommend we don t fully delete the field in the next Pants release At worst make it a no op and then deprecate the field < bitter ability 32190> I vaguely remember you have an option so that running a `pex binary` runs it the old way where we run from loose sources right If so then I think we could still have `experimental run in sandbox` do useful things when the old semantics are used The intent with the name `experimental ` is more than anything to discourage people from relying on the feature

bitter-ability-32190 · Answer

With the new change the user could should be running the python source in which case the semantics of in repo with an option to run in sandbox as well

happy-kitchen-89482 · Answer

I m not sure that we need to deprecate that field There may be reasons for people to want to build a pex and run it inline

bitter-ability-32190 · Answer

< happy kitchen 89482> I m not sure I understand what that means dizzy face The code being run is packaged into the PEX it is neither in a sandbox or in repo

bitter-ability-32190 · Answer

gt At worst make it a no op and then deprecate the field Ah I missed that I don t like the gymnastics this switchover is going to take in the code but in Pants 2 16 or whatever hopefully it ll be behind us

bitter-ability-32190 · Answer

I also dont want to inundate our users with too many new knobs levers

happy-kitchen-89482 · Answer

Oh right makes sense We would be changing the implementation of run in that case

bitter-ability-32190 · Answer

Yes however with how its implemented the change in behavior is opt in No one will upgrade and stumble into the new behavior they have to issue their command with a different spec than before

happy-kitchen-89482 · Answer

Is that always the case people could have been using the path to target spec before

bitter-ability-32190 · Answer

Then we d run it in the sandbox as we did before your change <https github com thejcannon pants blob d7589fc1cc4d746b51f9faff06ea80904b41f8a2 src python pants backend python goals run pex binary py L37>

happy-kitchen-89482 · Answer

Well but previously we weren t building the sources into the pex and now we would be

happy-kitchen-89482 · Answer

IIUC

bitter-ability-32190 · Answer

Ah yes And here s my trick <https github com pantsbuild pants blob d7589fc1cc4d746b51f9faff06ea80904b41f8a2 src python pants engine internals specs rules py L487> So ultimately If you use the pex binary target address behavior wont change You get warned and theres an option to switch to new behavior if you want If you use the file as your spec same as the above We dont issue an ambiguity error and we purposefully choose the binary to run If you re using a file address and theres no binary this is a new thing so whatever goes

bitter-ability-32190 · Answer

So any consensus

bitter-ability-32190 · Answer

I just don t want us to decide by not deciding sweat smile

happy-kitchen-89482 · Answer

I think leaving the field as is or renaming it and doing a deprecation cycle is fine

bitter-ability-32190 · Answer

Ah well I m bummed that this won t make it in 2 13 for our users and for my own repo As an aside after an upgrade to 2 14 if a user runs ` pants run path to file py` and has `run in sandbox` set they ll 3 get warnings see no evil

happy-kitchen-89482 · Answer

We can backport no

happy-kitchen-89482 · Answer

To 2 13

bitter-ability-32190 · Answer

What do you mean

happy-kitchen-89482 · Answer

Re gt Ah well I m bummed that this won t make it in 2 13 for our users and for my own repo

bitter-ability-32190 · Answer

Are you suggesting we cherry pick the feature from `main` to 2 13 And then in 2 13 add but also deprecate `run in sandbox` field

happy-kitchen-89482 · Answer

Oh I see what you mean If we get it all in in 2 13 we don t need to deprecate and can just remove it I m fine with that

bitter-ability-32190 · Answer

So I know < hundreds father 404> had reservations

hundreds-father-404 · Answer

my fear was that 2 13 already has so much in it and this is a big change

bitter-ability-32190 · Answer

I can help us toss something else out joy

happy-kitchen-89482 · Answer

I m not well positioned to chime on on that so I will have to remain agnostic on the question of whether we get this into 2 13 or 2 14 but I m pro the change either way

hundreds-father-404 · Answer

I guess 2 13 doesn t have a ton of user API changes The big ones are new CLI specs `filter` goal deprecated which impacts scripting etc So this seems fine from a User API disruption perspective Lots of internal changes tho so it may delay time to stabilizing the release And < bitter ability 32190> I still think we need to make the followup changes like what to do with ` tailor pex binary targets` in the same release It s coupled

bitter-ability-32190 · Answer

I can work on tailor ASAP

bitter-ability-32190 · Answer

This also is a big win for the debug stuff im adding because sandboxing the sources makes debugging silently impotent

hundreds-father-404 · Answer

did we decide what to do with `tailor`

bitter-ability-32190 · Answer

Too many threads see no evil

bitter-ability-32190 · Answer

My stance was is Python is way more likely to have people s scripts than packaged deliverables and theres no easy way to tell the latter So lets nix it and let `pex binary` be intentional and opt in

happy-kitchen-89482 · Answer

Do you mean tailor generates pex binary targets should be opt in Or we remove that functionality entirely from tailor I m pro the former against the latter because people have relied on tailor for this

bitter-ability-32190 · Answer

As long as it isnt on by default I m game

happy-kitchen-89482 · Answer

I think that makes sense

happy-kitchen-89482 · Answer

We already have an <https www pantsbuild org docs reference python section tailor pex binary targets|option for this> we just need to change the default

bitter-ability-32190 · Answer

Alrighty here s the current Plan of Attack My change s goes into 2 13 You can `run` a `python source` A handful of deprecation removal warnings regarding current and future behavior New field on `python source` and friends `run goal use sandbox` defaulting to True to maintain backwards compat < sparse lifeguard 95737> is this sufficient We mused about a global default a la `pex binary defaults` but I d like to avoid that with 2 14 s ` default `s feature

hundreds-father-404 · Answer

The key insight < bitter ability 32190> and I had yesterday is we re being extra careful to remove risk of backwards compatibility breaking For example we believe `run in sandbox=False` is probably the best default but we are no longer immediately jumping to that as it would be breaking As a result I m less concerned than I was before

bitter-ability-32190 · Answer

tailor PR <https github com pantsbuild pants pull 15962>