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.

Sign up for GitHub

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

Jump to bottom

Upgrade to Ruby 2.4.4 (fixes #2824) #3253

Merged
merged 25 commits into from
Aug 27, 2018
Merged

Upgrade to Ruby 2.4.4 (fixes #2824) #3253

merged 25 commits into from
Aug 27, 2018

Conversation

icarito
Copy link
Member

@icarito icarito commented Aug 22, 2018 •
edited
Loading

I believe this should pass all tests now. I'ts building every 15 minutes at https://jenkins.laboratoriopublico.org/job/Plots-Test/ .
When it is consistently green we'll have fixed #2824. Very close now!

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • PR body includes fixes #0000-style reference to original issue #
  • ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays.

Thanks!

icarito added 17 commits August 10, 2018 15:47
@ghost ghost assigned icarito Aug 22, 2018
@ghost ghost added the in progress label Aug 22, 2018
@plotsbot
Copy link
Collaborator

plotsbot commented Aug 22, 2018 •
edited
Loading

2 Messages
📖 @icarito Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.
📖 It looks like you haven’t marked all the checkboxes. Help us review and accept your suggested changes by going through the steps one by one. If it is still a ‘Work in progresss’, please include ‘[WIP]’ in the title.

Generated by 🚫 Danger

@icarito
Copy link
Member Author

icarito commented Aug 22, 2018

I believe we're back to green let's monitor https://jenkins.laboratoriopublico.org/job/Plots-Test/buildTimeTrend if it sticks let's merge this!

@icarito
Copy link
Member Author

icarito commented Aug 22, 2018

This PR includes a tweak to travis config that should make it run quicker! (Remove unneeded bundle install).

@icarito icarito changed the title [WIP] Upgrade to Ruby 2.4.4 (fixes #2824) Upgrade to Ruby 2.4.4 (fixes #2824) Aug 22, 2018
This was referenced Aug 22, 2018
Closed
Closed
@jywarren
Copy link
Member

Wow, how cool. here's the saved time, i think, in the graph -- some failures after that but then it stabilized (prob due to the additional commits?)

image

Are we good to merge then?

@jywarren
Copy link
Member

Does this also reflect an overall improvement in build time from ~20m to ~5m?

@icarito
Copy link
Member Author

icarito commented Aug 25, 2018

I worked yesterday on debugging why builds were taking longer and longer - this was kind of a stress test for our container setup. After adding tmp/ to .dockerignore we saved a bunch of time moving data around needlessly into the container.

Yes let's merge this! :-)

@jywarren
Copy link
Member

Ah but do we have to remove the regular tests in Jenkins first? Would this run a zillion (on every PR) if we merge it as it is? Or is that a setting from the Jenkins side?

@jywarren
Copy link
Member

Just want to clarify this last thing @icarito before merging. Tremendous work!

@icarito
Copy link
Member Author

icarito commented Aug 27, 2018

Jenkins is configured at the Jenkins side, currently it builds a specific branch (jenkins_strace).
Merging this won't affect Jenkins as it's configured at the Jenkins side.

@jywarren jywarren merged commit 57a6b04 into master Aug 27, 2018
@ghost ghost removed the in progress label Aug 27, 2018
@jywarren
Copy link
Member

Merged! Very hopeful here!

SrinandanPai pushed a commit to SrinandanPai/plots2 that referenced this pull request May 5, 2019
@icarito @jywarren
Upgrade to Ruby 2.4.4 (fixes publiclab#2824) (publiclab#3253)
4fe5dd4 
* Added strace option and artifact

* Restore Gitlab and add strace to Makefile

* Use strace within container

* Give more trace output

* Switch invocation of passenger to fork 9 processes in testing

* 256 string length. No op for triggering builds.

* Try to speed up by not running bower / bundler

* Build, then start.

* Built before

* Ignore logs in container

* Try with newer docker ruby image

* Set ruby version in Gemfile to 2.4.4

* Increase trace log to 512

* Don't export Redis port

* Don't export web app port for testing

* Set explicit Ruby version in Dockerfile

* Avoid running bundle in Travis

* Move strace to Gitlab to save disk space.

* Add strace and artifacts to tests.

* Remove redundant bundle and bower calls.

* Add capability to be able to strace on Gitlab.

* Run bower in Gitlab

* Remove bash

* Ignore tmp dir from docker container
@emilyashley emilyashley deleted the jenkins_strace branch January 15, 2020 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@icarito icarito

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Travis test gives error sometimes which gets removed by rebuild
3 participants
@icarito @plotsbot @jywarren