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

infra: accelerate builds #693

Merged
merged 15 commits into from
May 21, 2020
Merged

Conversation

codeboten
Copy link
Contributor

@codeboten codeboten commented May 14, 2020

Some build time improvements:

  • split lint/docker-tests/docs into their own steps. Since lint is usually the thing that fails anyways, it's good to have it run first. We could make the build depend on this step to prevent slowing other builds waiting in the pipeline (since we only have 5 workers)
  • move all pip install commands into a single line per test environment. this reduces the overhead of calling the pip command separately multiple times per environment.
  • removed pip upgrade command. This one I'm not sure about, it could potentially lead to broken builds in the future, but its taking roughly 10s each time it's done, which adds up as we add more extensions.

this should reduce the build times for py38
@codeboten codeboten added WIP Work in progress build & infra Issues related to build & infrastructure. labels May 14, 2020
@codeboten codeboten requested a review from a team May 14, 2020 07:01
@codeboten codeboten changed the title [WIP] infra: split lint/docker-tests/docs infra: accelerate builds May 15, 2020
@codeboten codeboten removed the WIP Work in progress label May 15, 2020
@toumorokoshi
Copy link
Member

Cool! I'd probably say the overhead of running pip multiple times is not a big issue (Python vm startup ~100ms, maybe an additional 100ms to load the core bytecode). But the lint re-order makes a lot of sense! Nice stuff.

Copy link
Member

@toumorokoshi toumorokoshi left a comment

Choose a reason for hiding this comment

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

approving assuming build will pass. Thanks!

tox.ini Outdated
@@ -164,26 +160,16 @@ changedir =

commands_pre =
; Install without -e to test the actual installation
python -m pip install -U pip setuptools wheel

py3{4,5,6}: python -m pip install -U pip setuptools wheel
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why we still need this line if it is run in travis.yml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, upgrading pip in travis before bringing up virtualenvs via tox was part of a failed experiment. I've reverted the change in the travis file, and kept the pip install -U command for the versions of python that needed it.

@codeboten codeboten requested a review from lzchen May 21, 2020 16:13
Copy link
Contributor

@lzchen lzchen left a comment

Choose a reason for hiding this comment

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

LGTM

@codeboten codeboten merged commit ae533f7 into open-telemetry:master May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build & infra Issues related to build & infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants