Pants: Have PR go through the equivalent of "Deploy pants pex unstable"

Created on 1 May 2019  路  11Comments  路  Source: pantsbuild/pants

Problem

Currently PR does not and cannot trigger Deploy pants pex unstable as a merged change on master does (e.g. at the bottom of https://travis-ci.org/pantsbuild/pants/builds/520546720)

This caused an issue where https://github.com/pantsbuild/pants/pull/7186 (Updating pex) broke Deploy pants pex unstable portion.

Proposed Solution

To prevent future breakages, I'd like to propose that we run the equivalent of Deploy pants pex unstable in PR builds. Given the security concern, we can publish the wheels to a separate and untrusted place, different from where master would publish the wheels to.

One issue might be how we can store the credentials for the separate s3 bin, but we can discuss how once folks are okay with the idea.

@jsirois @benjyw thoughts?

Most helpful comment

OK, assuming this is a good faith non end run then, it seems building the PEX and testing it runs is enough. No need to store it anywhere.

The issue is that the wheel builder shards produce something that would need to be consumed by the pex builder shards.

I think John is probably right? The "Deploy pants pex unstable" shards can consume the artifacts from shard 1-4, but does not need to publish the final pex.

All 11 comments

We've had _many_ discussions about burning binaries on PRs in the past. I am generally against it. Twitter generally wants it. I want to confirm first this isn't an end run around those discussions. Last I recall Twitter had agreed documenting its requirements for it's build process publically would be a sane next step. IIUC that never happened.

Last I recall Twitter had agreed documenting its requirements for it's build process publically would be a sane next step. IIUC that never happened.

We've been able to unspend a lot of technical debt on this internal build/deployment process in the past year+, so we may be in a better position to answer this now.

Also related to the build/deploy process internally:

There are some related things that I would like to upstream (like performance testing), but @illicitonion and I were pretty unsure we could transfer our internal performance testing to upstream infrastructure due to the difficulty of getting reliable hardware specs (see tweet). I was thinking one alternative might be to look into how to account for hardware variance if there are known, good methods that other open source projects use to setup performance testing, especially in CI.

Performance testing is semi-frequently an investigation that blocks our internal release, so (all imho) it could help to close the loop on fixing performance regressions in upstream pants in general if we could move any performance regression testing into upstream pants. But it's not clear how we'd want to do that / which use cases we would want to benchmark upstream.

We've had _many_ discussions about burning binaries on PRs in the past. I am generally against it. Twitter generally wants it. I want to confirm first this isn't an end run around those discussions. Last I recall Twitter had agreed documenting its requirements for it's build process publically would be a sane next step. IIUC that never happened.

The pants release process produces pexes that are hosted on github, and the shards in master are intended to test that that process does not break (afaik). Currently, those shards can break post-merge to master. That is the key motivation here.

The relevant issue in the most recent case was #7593.

Yes, it is the case that relatively few consumers (if any, other than Twitter) use the pexes to consume pants, but continuing to head in the direction of dogfooding pex by releasing pants with it seems like a good direction.

OK, assuming this is a good faith non end run then, it seems building the PEX and testing it runs is enough. No need to store it anywhere.

OK, assuming this is a good faith non end run then, it seems building the PEX and testing it runs is enough. No need to store it anywhere.

The issue is that the wheel builder shards produce something that would need to be consumed by the pex builder shards.

from op @wisechengyi:

Given the security concern, we can publish the wheels to a separate and untrusted place, different from where master would publish the wheels to.

Are the security requirements different than e.g. for the output of the bootstrap shards? It seems like there might be more artifacts we need to be publishing -- is that relevant, or is it some other reason? I cannot tell if there's context I'm missing here.

OK, assuming this is a good faith non end run then, it seems building the PEX and testing it runs is enough. No need to store it anywhere.

The issue is that the wheel builder shards produce something that would need to be consumed by the pex builder shards.

I think John is probably right? The "Deploy pants pex unstable" shards can consume the artifacts from shard 1-4, but does not need to publish the final pex.

Are the security requirements different than e.g. for the output of the bootstrap shards? It seems like there might be more artifacts we need to be publishing -- is that relevant, or is it some other reason? I cannot tell if there's context I'm missing here.

Same as bootstrap I think, but turns out maybe we don't need to do that as described ^

Currently aim to work on this next week. I haven't looked the travis build for a while, so will circle back if things are not what I expected.

Closing as being stale. Please feel free to reopen if this is still desired and anyone is interested in working on it.

Was this page helpful?
0 / 5 - 0 ratings