Circle ci use prebuilt docker image

somaticio / tensorflow.rb

tensorflow for ruby

BSD 3-Clause "New" or "Revised" License

829 stars 63 forks source link

Circle ci use prebuilt docker image #37

Closed nethsix closed 8 years ago

nethsix commented 8 years ago

DON'T MERGE YET, instead:
1. Please sign up for CircleCI (click 'Add' button on the top-right) when you have time here: https://github.com/integrations/circle-ci
2. At the dashboard, select yourself
3. Select ruby-tensorflow
4. Look for this branch, and click on it
5. Click 'rebuild' at the top
6. Let us know in the comments if it was successful
Notes:
- Changed circle.yml to instruct CircleCI to use prebuilt docker image for building ruby-tensorflow and running specs
- It now takes < 4 mins instead of ~45 mins to build and run specs
- Auto-CI is still turned off since we're testing the flow

nethsix commented 8 years ago

@Arafatk @chrhansen @geoffreylitt Nice! There's a check mark by CircleCI for this branch! If you guys think 3 mins is an acceptable time for CI to run for each pull request I'll change circle.yml to turn on checks for all pull requests. Please also let me know if there's anything we can improve on.

geoffreylitt commented 8 years ago

@nethsix nice! This makes a lot of sense to me. Given the time needed to set up dependencies, it seems like a prebuilt image is the only reasonable option for performance.

One small question: is there a reason not to include the commands on separate lines in the circle.yml? I think it would be more readable than having all on one line.

chrhansen commented 8 years ago

@nethsix and add this to the top of README.md, like here:

`Linux CPU`	`Linux GPU PIP`	`Mac OS CPU`
	Not Configured	Not Configured

Code:

|  **`Linux CPU`**   |  **`Linux GPU PIP`** | **`Mac OS CPU`** |
|-------------------|----------------------|------------------|----------------|
| [![Build Status](https://circleci.com/gh/Arafatk/tensorflow.rb.svg?style=shield)](https://circleci.com/gh/Arafatk/tensorflow.rb) | _Not Configured_ | _Not Configured_ |

nethsix commented 8 years ago

@geoffreylitt Thanks for sharing your opinion and suggestion. Yeah, having them on multiple lines is definitely better; I will try to do that as well. The problem that I am facing now is the end result is not saved in the docker image between each 'docker run' execution. Probably need to figure out how to get CircleCI to do 'docker commit' after each 'docker run'. Any suggestions?

@chrhansen Thanks!!!!!

chrhansen commented 8 years ago

@nethsix I think 3 min is fully acceptable, let's turn it on for all branches, so there's no doubt the importance of tests. If someone (or you) feels eager down the road they can optimize the build to be more efficient.

arafatkatze commented 8 years ago

@nethsix This is really nice, 3 mins is perfectly fine and it would be great to have the check on for all pull requests.

Thanks a lot for this !!

nethsix commented 8 years ago

@Arafatk @chrhansen. Got it! I'll enable auto CI for all pull requests.

chrhansen commented 8 years ago

@Arafatk I think this PR was still missing the inputs from @geoffreylitt and adding the build-badge.

arafatkatze commented 8 years ago

@chrhansen I am sorry, I did it hurry. I emailed @nethsix about this and he will send a new pr soon. Sorry for the confusion.

Also, I am experiencing an issue when I was experimenting with #32 . Here in the latest commit, I am making no changes,but circle ci fails which is weird.

nethsix commented 8 years ago

@Arafatk I left a detailed comment about this on the issue thread itself (https://github.com/Arafatk/tensorflow.rb/pull/32). In summary, that branch has no circle.yml file to tell CircleCI how to perform CI, thus CircleCI failed to build dependencies correctly.