#general
Title
w
wide-energy-11069
10/26/2018, 11:38 PM@enough-analyst-54434
Recent bash to python conversion on release.sh caused some complication for us internally, since it will call now pants thus requiring bootstrap itself via internet.
I was wondering if we can just push a pex directly per master SHA to binaires.pantsbuild.org, as it should benefit the community as well to consume any SHA with much lower overhead. We are already doing so per tag, so basically just increasing the frequency there.
Thoughts?
w
witty-crayon-22786
10/26/2018, 11:46 PMi would be in favor of this.
when John added the final deploy shard for github, we resolved https://github.com/pantsbuild/pants/issues/4896 (edit: which was 100% reasonable, to be clear)
i think that doing an s3 pex deploy per master/branch commit would be cheap and worthwhile.
(...lot o' history in 4896... cc @gorgeous-alligator-94763)
e
enough-analyst-54434
10/27/2018, 12:22 AMThe community does not consume master commits. Twitter, one important member of the community does. This is just calling out a - likely innocent - disingenuous argument. Not to mention foisting by one's own petard.
That said, have at it. I'm loosing some of my fight on this issue.
w
wide-energy-11069
10/27/2018, 4:53 AMcool thanks for the understanding. maybe it could be a thing that if we build it, people would come 🙂
👍 1
w
witty-crayon-22786
10/27/2018, 3:40 PMYeah. I could imagine that with advertising, people would use them every now and then.
w
wide-energy-11069
10/28/2018, 11:11 PM@enough-analyst-54434 qq: Does https://github.com/pantsbuild/pants/blob/38ccf42c49f31e26462a266e8c63fb7875bcec14/.travis.yml#L131-L152 deploy
pants.pexOR branch=masterif: Copy code
stages:
- Test Pants
- name: Deploy Pants Pex
if: tag IS present AND tag =~ ^release_.*$e
enough-analyst-54434
10/29/2018, 12:27 AMNo, it builds the multiplatform pex (
./build-support/bin/release.sh -pAlso,
PANTS_PEX_RELEASE=stablew
wide-energy-11069
10/30/2018, 5:09 AMSo we realized that the pex built in OSS is a thin pex, as in only
pantsbuild.pantspants.pexpantsbuild.pantsg
gorgeous-alligator-94763
10/30/2018, 6:36 AMI’m thinking of adding an option that we would pass into the release script that can only be used with -p and -q that will build a fat pex. how does that sound?
e
enough-analyst-54434
10/30/2018, 6:51 AM-p already is fat, what do you mean exactly?
w
wide-energy-11069
10/30/2018, 3:02 PMIt does not contain the wheels needed for pycodestyle for example.
w
witty-crayon-22786
10/30/2018, 3:15 PMthere is an environment variable you can pass to include more contrib modules
but yea, if you were going to do
thinthicke
enough-analyst-54434
10/30/2018, 3:33 PMSo, it would be awesome to change one thing at a time. Today we publish a "thin" pex by your definition for tags. You wanted this on master too. And prior to that you wanted a simple tool to build these for any branch to use for internal testing. Can we knock off those 1st before deploying fat pexes which is something we currently do nowhere?
w
wide-energy-11069
10/30/2018, 3:35 PMbasically push through https://github.com/pantsbuild/pants/pull/6694 first right?
e
enough-analyst-54434
10/30/2018, 3:37 PMYes, and also just do what was discussed. A fat pex - if agreed it even made sense - must include all contribs and would 1st require the groundwork to turn off all contribs by default if you want to claim this is useable by anyone.
Just noticed:
(Maybe benjy can offer build-pants-pex-as-a-service :p)I truly am dumbfounded. I thought the whole / only point of Twitter wanting to consume publically released dists / pexes was that they were the actual ones to go through CI. If you want to build an alacart pex using pre-tested wheels, you already can but that exact combo of wheels in 1 pex will not be the binary that ran ci.
IOW, modifying release.sh -p to take args so that
release.sh -p contrib.x contrib.yTwitter or anyone else could run this to generate their alacart pex of CI built / tested component parts and then deploy that however they wished.
With that 30 delta line change done, the last step as I understand it is a bit of script to run said-same on an OSX machine but using locally built wheels using
release.sh -nThats the bit that supports roundtripping through Twitter internal CI before a PR hits master.
w
wide-energy-11069
10/30/2018, 3:51 PMthere’s already
PANTS_PEX_PACKAGESe
enough-analyst-54434
10/30/2018, 3:53 PMAh right. So why are deployed pexes needed again if Twitter has to use that env var? That implies you rebuild a pex anyhow.
w
wide-energy-11069
10/30/2018, 3:57 PMI just learned that, but yes, we did rebuild a pex via
PANTS_PEX_PACKAGESrelease.sh./pants ...e
enough-analyst-54434
10/30/2018, 3:57 PMBut you added shelling out to pants - so don't do that
g
gorgeous-alligator-94763
10/30/2018, 3:58 PMmy use case is a little different here
e
enough-analyst-54434
10/30/2018, 3:58 PMWhatcha got?
w
wide-energy-11069
10/30/2018, 3:59 PMshelling out to pants - so don’t do thatyeah, initially I took on the task to convert some of the bash to python, but it does not seem to work well, hence reverted.
e
enough-analyst-54434
10/30/2018, 4:00 PMpython would be fine, it sounds like your requirement is pure python that needs to fetch no deps
... except from s3 - that's ok for some reason
presumably a network proxy whitelist that bans pypi
g
gorgeous-alligator-94763
10/30/2018, 4:01 PMI want to build linux wheels (for now) that don’t go into S3 and I was hoping I don’t need to change the release script (for building the pex with the contents of dist instead of S3). I need the fat pexes to be able to run the pex as the pants executable.
w
wide-energy-11069
10/30/2018, 4:01 PMwe whitelisted s3, for pypi we have internal mirror. the choke point is rust deps, which contains some forks of repos
g
gorgeous-alligator-94763
10/30/2018, 4:01 PMit seems like my best path forward is going back to my original solution which is what @enough-analyst-54434 and I spoke about
e
enough-analyst-54434
10/30/2018, 4:02 PM@gorgeous-alligator-94763 Why did that get derailed as a solution in the 1st place?
@wide-energy-11069 - gotcha. So python replacement of the release script can be total leveraging libs - say requests or boto, it just can't use pants code
Actually - it almost certainly can use pants code, but via PYTHONPATH=src/python aka sourcing the pants_venv
g
gorgeous-alligator-94763
10/30/2018, 4:04 PMbecause @wide-energy-11069 was making changes to the release script at the same time, so to avoid clashes, I tried to find another way to do it. I thought it would simpler to use -q since we only needed a linux compatible pex
e
enough-analyst-54434
10/30/2018, 4:05 PMCase in point on the latter is Danny's reworking of build-support/bin/download_binary.sh
That gets executed before rust is bootstrapped so is a proof in point
Gotcha @gorgeous-alligator-94763.
w
wide-energy-11069
10/30/2018, 4:06 PMpip install everythingjust not building the rust code
e
enough-analyst-54434
10/30/2018, 4:07 PMGreat - Danny's technique can be mimicked then
if you want to detour in a rewrite.
I'd suggest maybe doing that after though as a cleanup. None of the things we talked about is hard or large in terms of ammendments to the existing bash
w
wide-energy-11069
10/30/2018, 4:09 PMto clarify, were you suggesting using
build-support/bin/download_binary.she
enough-analyst-54434
10/30/2018, 4:09 PMNo
I'm suggesting you use the technique used in that script to run pants code
Danny should be a great resource. And you can teach him about Fire - which he missed in that change despite a prod.
w
wide-energy-11069
10/30/2018, 4:11 PMOn a high level, I was just confused how this would work without building the rust code. Sure I will talk to Danny 🙂
e
enough-analyst-54434
10/30/2018, 4:11 PMAny pants code which does not invoke the engine doesn't need rust is the short answer - so basically all utils and subsystems are useable.
w
wide-energy-11069
10/30/2018, 4:13 PMthanks!
w
witty-crayon-22786
10/30/2018, 4:13 PMno BUILD file parsing
which means no binaries can be built
so would not work in this case.
e
enough-analyst-54434
10/30/2018, 4:14 PMYeah - aka don't run pants
w
witty-crayon-22786
10/30/2018, 4:14 PMYi is going to open a ticket
ah, i see what you mean.
e
enough-analyst-54434
10/30/2018, 4:14 PMOK. His prior attempt didn't try to build binaries iirc - just use Fetcher
w
witty-crayon-22786
10/30/2018, 4:14 PMyea, Yi's going to open a ticket
w
wide-energy-11069
10/30/2018, 4:18 PMHis prior attempt didn’t try to build binaries iirc - just use Fetcherthe
run_local_pantse
enough-analyst-54434
10/30/2018, 4:20 PMUnderstood - pointing out that was an arguably extravagant way to run a main that you didn't need to do.
With the venv activated you can just run it
w
witty-crayon-22786
10/30/2018, 4:20 PM@wide-energy-11069: let's do this on the ticket please. i think i understand what John is suggesting, but easiest in that format.
👍 1
e
enough-analyst-54434
10/30/2018, 4:21 PMI'll underscore there was nothing new / interesting in this thread besides fat. That is totally new and something we've never done for pypi dists or pexes.
It would probably be most helpful to just lay out Twitter's requirements explicitly so we can end this once and for all with a solution. IE: We can whitelist up to 2 urls, we can mirror x but not y, and we want the produced binaries to have been used in XYZ ways in CI.
w
witty-crayon-22786
10/30/2018, 4:26 PMyea, that is what i hope to add to Yi's ticket if it doesn't include it.
e
enough-analyst-54434
10/30/2018, 4:26 PMExcellent
w
witty-crayon-22786
10/30/2018, 4:35 PM(and Dorothy's, which is for a different usecase)
e
enough-analyst-54434
10/30/2018, 4:37 PMYeah - good. The vet a branch internally pre hitting pantsbuild/pants:master
w
wide-energy-11069
11/27/2018, 1:05 AMslight follow up: we were able to quickly narrow down an offending commit via the master pexes which caused openssl failure only on newer version of the linux (ubuntu 18 in this case)
e
enough-analyst-54434
12/06/2018, 6:58 PMThis could have been caught publically w/o the master pexes: https://github.com/pantsbuild/setup/pull/29
w
witty-crayon-22786
12/06/2018, 7:00 PMI think what he meant was that he was able to bisect to the precise commit more quickly
e
enough-analyst-54434
12/06/2018, 7:00 PMYes, but if that CI was run as part of our OSS process - no bisect
It would just be red right away
w
witty-crayon-22786
12/06/2018, 7:00 PMSince this was a usecase that required a Centos binary being consumed on Ubuntu
e
enough-analyst-54434
12/06/2018, 7:00 PMIt does not
apparently
w
witty-crayon-22786
12/06/2018, 7:00 PMThe bug did
e
enough-analyst-54434
12/06/2018, 7:01 PMException message: libssl.so.10: cannot open shared object file: No such file or directory
?
Thats what the linked setup CI gets for ubuntu 12.04 and 14.04 straight up
w
witty-crayon-22786
12/06/2018, 7:01 PMright
because it was consuming a binary built by Centos 6... ie, in pantsbuild/pants CI
so bisecting would mean rebuilding things on centos and then consuming them on ubuntu... non trivial
(easier if you're on linux though)
or i suppose you could do it with two docker images on osx
e
enough-analyst-54434
12/06/2018, 7:03 PMI'm confused, but I'll file an issue to make the setup CI part of release. I'm just showing the master pex releases have nothing to do with detecting this - we could have been detecting from the wheels already as my PR proves.
w
witty-crayon-22786
12/06/2018, 7:04 PMoooh, i see what you mean.
e
enough-analyst-54434
12/06/2018, 7:04 PMYeah - batting down another poor argument!
I'll see if I can figure something out: https://github.com/pantsbuild/pants/issues/6875
w
witty-crayon-22786
12/06/2018, 7:07 PMi think that if it were possible to bisect directly with the
setupin this case, the bug was so easy to detect in the right env (centos6 -> ubuntu) that just running setup would be sufficient. in other cases might be harder
but we are not consumers of the setup script in general, and in the past we've talked about switching setup to consuming pexes... not sure whether that is still a goal
e
enough-analyst-54434
12/06/2018, 7:10 PMsetup wheel/pex aside - my point here is there should be no need for private bisects - we should publically test the case whenever possible. Private testing is wonderful to have but horrible to rely on.
As far as setup consuming pexes - no one is clamoring ofr this that I know of so it hasn't hit my priority radar at any rate.
w
witty-crayon-22786
12/06/2018, 7:11 PMyep. insofar as it is possible to test wacky combinations like this, we're all for it.
e
enough-analyst-54434
12/06/2018, 7:12 PMCool. And this is not whacky - Ubuntu is still - I assume - the most popular distro out there.
w
witty-crayon-22786
12/06/2018, 7:12 PMunfortunately, it probably generalizes to the thing you were trying to test before: build a binary once within a travis run, and then kick off all of the other shards to consume that binary
re: ubuntu, yes: but the cross-building thing was unexpected.
e
enough-analyst-54434
12/06/2018, 7:13 PMYeah - and that proved strictly slower.
w
witty-crayon-22786
12/06/2018, 7:13 PMi thought that the issue was that PR builds couldn't actually use creds to write anywhere? or did you find a way around that
(we added OSX 10.12 and 10.13 sanity-check shards recently, but it's not clear that they're adding as much coverage as they could be, because it's not a cross-compile from some other OSX)
e
enough-analyst-54434
12/06/2018, 7:14 PMThere is no way around that ever for any CI provider.
w
witty-crayon-22786
12/06/2018, 7:16 PMwhich has been nagging at the back of my brain from a remoting perspective for a while =/ (cc @average-vr-56795)
how do you give PR contributors access to a remoting cluster? feels like a whitelist is the only thing
or bring your own creds to test with, which is not very equitable
e
enough-analyst-54434
12/06/2018, 7:18 PMhow do you give PR contributors access to a remoting cluster?You prop up a github app. IE: you can't cheese off travis or circle, etc.
w
witty-crayon-22786
12/06/2018, 7:23 PMit's expensive cheese... and in theory those providers are already plenty exploitable
but yea.
hm... or maybe ... if the provider implemented an authenticated proxy or something?
requests from within the container are accepted without auth, and are then authed to another (gRPC) service
e
enough-analyst-54434
12/06/2018, 7:27 PMWhatever it is, it just has to base all trust on github webhooks - it cannot use credentials configured by the main repo PR writers fork from.
w
witty-crayon-22786
12/06/2018, 7:27 PMyea.
you could have some semblance of protection if the github webhook api provided something that was signed by their private key for a particular PR
then you could require that signature for a PR that is less than X hours old or something
anyway, back to options parsing =P
e
enough-analyst-54434
12/06/2018, 7:31 PMgithub already passes a secret to your webhook to prove github is github. From there you trust the payload and mask it againts any internal whitelists of parent repos you're allowing to use your service. This much is all solved and it's what travis, circle, etc use.
w
witty-crayon-22786
12/06/2018, 7:32 PMmhm
neat, thanks
5 Views