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 CSV2Avro #3

Closed
wants to merge 5 commits into from
Closed

Dockerize CSV2Avro #3

wants to merge 5 commits into from

Conversation

peterableda
Copy link
Contributor

No description provided.

RUN mkdir -p /usr/src/app
WORKDIR /usr/src/app

COPY Gemfile* *.gemspec /usr/src/app/
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not use globs in the COPY commands and have individual steps for Gemfile, Gemfile.lock and csv2avro.gemspec. This will ensure that the intermediate steps can and will be cached. Probably most ideal would be:

COPY Gemfile /usr/src/app
COPY Gemfile.lock csv2avro.gemspec /usr/src/app

If this is valid.

@rgabo
Copy link
Contributor

rgabo commented Mar 25, 2015

@peterableda you should also add ENTRYPOINT/CMD to the image and point it to bin/csv2avro.

@peterableda
Copy link
Contributor Author

@rgabo Yes I know I should create a CMD but I got stuck with that. I haven't figured out how will I parametrize the cli.

@@ -8,7 +8,7 @@ options = {}
OPERATIONS = %w[convert]

option_parser = OptionParser.new do |opts|
opts.banner = "Version #{CSV2Avro::VERSION} of CSV2Avro\n" \
opts.banner = "Version 0.1.0avro of CSV2Avro\n" \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the avro is unnecessary

Copy link
Contributor

Choose a reason for hiding this comment

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

To be honest, I would not include the version number here. I've randomly checked json2csv, in2csv and sql2csv and none of them have a banner, just show usage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked a few too... q, avro-tools has a banner :D

@peterableda peterableda closed this Apr 7, 2015
@rgabo rgabo deleted the feature/docker branch April 28, 2015 15:26
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