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

Adding Pip cache directory to replace special cache directive. #5281

Merged
merged 1 commit into from
Jul 13, 2018

Conversation

hoangviet1993
Copy link
Contributor

Explanation

It has been a few days after my #5244 was merged and yet, Travis was still installing codecov and pyyaml on start. Pics from build 11932.1 on develop:
screen shot 2018-07-13 at 11 21 04 am
This means that codecov and pyyaml was not cached at all.

In the spirit of careful investigation for every line of code written, I dig up the related #1640 and its parent issue #1636. Toward the end of #1636 thread, pip cache was dropped due to instability and ruled ineffective due to variable time.

We have since grown from running 3 jobs to 13 jobs. Every seconds saved is tangible imo. I started to look into why pip modules are not getting cached. Here is my finding. I then made the changes and tested on my own Oppia's Travis. Here is pic from my Travis's job 138.1 of branch viet_branch_5222:
screen shot 2018-07-13 at 11 28 49 am
As you can see, cached pyyaml shaved 8 secs off that one job. 8 x 13 = 104 secs, less than 2 minutes for each PR. In the grand scheme of things, 2 minutes are not a lot but is still worthy of a PR for me!

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes.
  • The PR explanation includes the words "Fixes #bugnum: ...".
  • The linter/Karma presubmit checks have passed.
    • These should run automatically, but if not, you can manually trigger them locally using python scripts/pre_commit_linter.py and bash scripts/run_frontend_tests.sh.
  • The PR is made from a branch that's not called "develop".
  • The PR follows the style guide.
  • The PR is assigned to an appropriate reviewer.
    • If you're a new contributor, please ask on Gitter for someone to assign a reviewer.
    • If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue.

@hoangviet1993 hoangviet1993 requested a review from apb7 July 13, 2018 15:35
@codecov-io
Copy link

codecov-io commented Jul 13, 2018

Codecov Report

Merging #5281 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5281   +/-   ##
========================================
  Coverage    47.41%   47.41%           
========================================
  Files          472      472           
  Lines        27172    27172           
  Branches      4197     4197           
========================================
  Hits         12884    12884           
  Misses       14288    14288

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3e3b95...12ad6c6. Read the comment docs.

Copy link
Contributor

@apb7 apb7 left a comment

Choose a reason for hiding this comment

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

Hi @hoangviet1993, I think we should give this a try. Let's see the end results once :)

@apb7 apb7 merged commit a7ab2c0 into oppia:develop Jul 13, 2018
@hoangviet1993 hoangviet1993 deleted the properPipCache branch July 14, 2018 02:23
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.

3 participants