Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor travis.yml to deduplicate Pyenv and AWS pex code #7397

Merged
merged 9 commits into from
Mar 19, 2019

Conversation

Eric-Arellano
Copy link
Contributor

Problem

Our .travis.yml is quite complex. While Mustache has helped with addressing this, we currently duplicate several intricate values like the command to publish pants.pex to AWS.

While working on pantsbuild/setup#39, a better design came up to de-duplicate individual script entries through anchors.

Solution

  • Introduce new PYENV_BIN env var to avoid having to type ${PYENV_ROOT}/bin/pyenv multiple times.
  • Define Pyenv env vars globally. They do not hurt anything if we don't use Pyenv on the shard, and allow us to deduplicate code such as which version of Py3 to install.
    • Key detail: we first check if $PYENV_ROOT is defined and only redefine if not. Travis's Ubuntu images use Pyenv to get multiple Python versions installed, so we must do this check to avoid overwriting what comes on the system.
  • Introduce new globally defined commands pyenv_setup, pyenv_install_py36 and pyenv_global_py36.
  • For bootstrapping pants.pex, introduce new globally defined commands aws_deploy_pants_pex and aws_get_pants_pex.
  • Only build fs_util on the Py27 shard.
    • OSX was building and deploying for every shard
    • Linux was building on Py36 even though we never deployed it.

@Eric-Arellano Eric-Arellano changed the title WIP: Refactor travis.yml to deduplicate Pyenv and AWS pex code Refactor travis.yml to deduplicate Pyenv and AWS pex code Mar 18, 2019
Originally I had this from the setup repo, but realized we can't use it and must use Mustache because we need the OpenSSL env vars to be included in the anchor for things like &py36_osx_env, and we couldn't mix this anchor with that new anchor definition.
Two pants.pex found for Py36 Linux :/
Copy link
Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks like a nice improvement.

It would be good moving forward to think about which components should be in scripts rather than embedded in the yaml, but these items seem to be below the threshold.

@Eric-Arellano Eric-Arellano merged commit 532dd95 into pantsbuild:master Mar 19, 2019
@Eric-Arellano Eric-Arellano deleted the refactor-travis-yml branch March 19, 2019 23:23
Eric-Arellano added a commit to Eric-Arellano/pants that referenced this pull request Mar 19, 2019
stuhood pushed a commit that referenced this pull request Mar 20, 2019
### Problem
Our `.travis.yml` is quite complex. While Mustache has helped with addressing this, we currently duplicate several intricate values like the command to publish `pants.pex` to AWS.

While working on pantsbuild/setup#39, a better design came up to de-duplicate individual script entries through anchors.

### Solution
* Introduce new `PYENV_BIN` env var to avoid having to type `${PYENV_ROOT}/bin/pyenv` multiple times.
* Define Pyenv env vars globally. They do not hurt anything if we don't use Pyenv on the shard, and allow us to deduplicate code such as which version of Py3 to install.
   * Key detail: we first check if $PYENV_ROOT is defined and only redefine if not. Travis's Ubuntu images use Pyenv to get multiple Python versions installed, so we must do this check to avoid overwriting what comes on the system.
* Introduce new globally defined commands `pyenv_setup`, `pyenv_install_py36` and `pyenv_global_py36`.
* For bootstrapping `pants.pex`, introduce new globally defined commands `aws_deploy_pants_pex` and `aws_get_pants_pex`.
* Only build `fs_util` on the Py27 shard.
   * OSX was building and deploying for every shard
   * Linux was building on Py36 even though we never deployed it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants