-
Notifications
You must be signed in to change notification settings - Fork 438
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
RethinkDB addon (WIP) #676
Conversation
def after_prepare | ||
sh.fold 'rethinkdb' do | ||
sh.echo "Installing RethinkDB version #{rethinkdb_version}", ansi: :yellow | ||
sh.cmd "service rethinkdb stop", sudo: true |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Not sure where the taintedness is coming into play, but with my fixes, that is not a problem when I run the specs locally. |
A possible fix for these issues: https://gist.github.com/BanzaiMan/20cc01553a1948236138 |
Thanks for the feedback and proposed fixes @BanzaiMan . I'll update the PR shortly. Do you have any suggestions or example code for how to check for the operating system? |
@danielmewes |
I applied your fixes @BanzaiMan and the tests are now passing. However I couldn't get the OS check to work. I tried checking |
@danielmewes How are you testing it? |
@BanzaiMan I was using the Vagrant file and running |
|
||
def after_prepare | ||
sh.fold 'rethinkdb' do | ||
#if ENV['TRAVIS_OS_NAME'] != 'linux' |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@danielmewes Do you need help? |
@BanzaiMan Thanks for your comments. Picking this up again now... |
@BanzaiMan Updated as suggested. This seems to be working now. |
What's the issue here? What needs to happen to get this merged? Who is this blocked with? |
|
||
def after_prepare | ||
sh.fold 'rethinkdb' do | ||
sh.if "ENV['TRAVIS_OS_NAME'] != 'linux'" do |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@danielmewes @BanzaiMan Who are we waiting on at this point? I am not sure if more changes are needed from @danielmewes or if @BanzaiMan needs to help fix something. Either way, this was supposed to be a quick solution back in March. It's now June... Thanks! |
Thanks for the explanation @BanzaiMan . RethinkDB works on Mac, but the setup procedure is entirely different and I don't want to deal with that now. |
@danielmewes Thanks for the update. I ran a quick test to install 2.3, but it is failing: https://staging.travis-ci.org/BanzaiMan/travis_staging_test/builds/493918#L143-L166
Could you review these messages? Also, do you want the build to fail immediately if installation fails for one reason or another? |
Ah, I think this is the same issue as travis-ci/travis-ci#4772 . I'll change the |
module Build | ||
class Addons | ||
class Rethinkdb < Base | ||
SUPER_USER_SAFE = true |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@BanzaiMan does this mean this will not work with the new infrastructure? @danielmewes requiring sudo makes this a lot less useful because it blocks it out of the new infrastructure. Goal is to make using rethink as easy as all the other built-in dbs available in Travis. |
@BanzaiMan Thanks for the comments. I'll look into those things. @hueniverse Both the MariaDB (which I based this on) and PostgreSQL addons also require sudo on the destination system. Do you have an example for a DB addon that works without it that I could look at? |
If the only requirement is |
Scratch that. |
@BanzaiMan So we are good as is with the sudo requirements? |
@hueniverse Yes, I think so. The version specification still needs to be fixed. |
Also a PR to docs, please. |
@BanzaiMan Can you point me where those docs are and a similar section to based this on? |
@hueniverse It should go into https://github.com/travis-ci/docs-travis-ci-com/blob/gh-pages/user/database-setup.md. (Which is rendered at https://docs.travis-ci.com/user/database-setup/). You can see similar sections (e.g., MariaDB) on that page. |
@BanzaiMan I fixed the |
Documentation is coming. |
Documentation is in travis-ci/docs-travis-ci-com#673 |
@danielmewes you're my hero! |
OK, this has been deployed to staging one more time. |
module Build | ||
class Addons | ||
class Rethinkdb < Base | ||
SUPER_USER_SAFE = false |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Thanks for testing and your comments @BanzaiMan . I turned |
Deployed to production now. |
Thanks again for your help in getting this into shape @BanzaiMan ! |
Thanks @BanzaiMan @danielmewes! Can't wait to try it out. |
This adds a RethinkDB addon.
The supplied test is currently failing as follows, and I'm not sure what that means:
Mentioning @joshk who suggested opening the pull request despite the remaining failures.