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

Dockerize ytrello #12

Merged
merged 5 commits into from
Feb 27, 2018
Merged

Dockerize ytrello #12

merged 5 commits into from
Feb 27, 2018

Conversation

teclator
Copy link
Contributor

@teclator teclator commented Feb 26, 2018

Just an idea, the ytrello credentials could be copied also to /root/.config/ytrello-creds.yml instead of using environment variables.

Copy your osc credentials as oscrc

cp ~/.oscrc /ytrello_path/oscrc

Build ytrello

docker build -t ytrello .

Add ytrello as an alias

# Add to your .bashrc a ytrello alias and define the environment trello credentials
export TRELLO_DEVELOPER_PUBLIC_KEY=ffffffffffffffffffffffffffffffff
export TRELLO_MEMBER_TOKEN=ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
alias ytrello="docker run -ti -e TRELLO_DEVELOPER_PUBLIC_KEY=$TRELLO_DEVELOPER_PUBLIC_KEY -e TRELLO_MEMBER_TOKEN=$TRELLO_MEMBER_TOKEN ytrello"

Start using it

ytrello ./check
ytrello ./create 1065...

Copy link
Owner

@mvidner mvidner left a comment

Choose a reason for hiding this comment

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

OK the deployment stage is something which I haven't really figured out, and this is another improvement to it.
Can you please move the docs in the PR description to the README (or perhaps a separate README-docker?)

ytrello.rb Outdated
$stderr.puts "Installing the needed Ruby gems failed"
exit 1
end
end
Copy link
Owner

Choose a reason for hiding this comment

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

Is this necessary for the dockerization? Because removing this would mean that the previous deployment method gets broken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main problem is that it assumes the bundler_dir for you and force the install if not present while it should just a

require 'rubygems'
require 'bundler/setup'

letting to the user the decision about his bundle path, config and so on.

http://bundler.io/v1.12/bundler_setup.html#bundlersetup

We could modify the Dockerfile to explicitly use the .vendor dir avoiding the remove of this chunk of code but not sure if it is the recommended way of bundle use. I was surprised why always that I launched the commands after a build it was installing again all the gems... took me sometime to realize that it was because the code force you to install the gems in a specific path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, I will just remove the global path and it will work fine again not having to remove anything from previous code by now.

@teclator teclator force-pushed the Dockerize branch 3 times, most recently from 5fb43a1 to 71cab00 Compare February 26, 2018 14:23
@mvidner
Copy link
Owner

mvidner commented Feb 26, 2018

Well, I can still make it fail if I first run ytrello in my checkout and THEN build the image. And it seems you cannot build it from a fresh checkout because it expects a Gemfile.lock. Can you please make it work with a fresh checkout?

@mvidner
Copy link
Owner

mvidner commented Feb 26, 2018

Maybe we should simply commit the Gemfile.lock

@teclator
Copy link
Contributor Author

@mvidner ups that's true, I overlooked that the Gemfile.lock was git ignored. So yes, I would add it to git or use at least add one for being copy into the container as Gemfile.lock

In the past it was usually ignored, but nowadays, being an application and not a library it usually makes sense to have the gems versions that we are sure work fine locked.

@teclator teclator force-pushed the Dockerize branch 3 times, most recently from 7f76ec6 to acb5092 Compare February 26, 2018 19:55
openSUSE Leap 42.3 does have ruby-2.4 (but the default is ruby-2.1)
@teclator
Copy link
Contributor Author

@mvidner last changes summary:

I was using an old Gemfile.lock and also the Dockerfile was based on ruby 2.2. I have adapted it using ruby 2.4 instead.

If you prefer, we can omit the Gemfile.lock and just remove the copy from the Dockerfile.

@mvidner
Copy link
Owner

mvidner commented Feb 27, 2018

Now I synced Gemfile and Gemfile.lock, but @lslezak 's bundler setup no longer works as nicely. You have to use

$ bundler.ruby2.4 install
$ ruby.ruby2.4 $ytrello_tool

Huh, I hope I can find a way out of this mess.

@teclator
Copy link
Contributor Author

That is exactly the problem about having multiple versions and what I tried to solve using docker. BTW, bicho was dropping support for ruby < 2.3 dmacvicar/bicho@8f23785

@mvidner
Copy link
Owner

mvidner commented Feb 27, 2018

OK, I think this still is an improvement, and if we need to touch this area again, let's make a regular gem out of it, and put it in OBS to get nicely versioned executables.

@mvidner mvidner merged commit 8a36d71 into mvidner:master Feb 27, 2018
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.

2 participants