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

Support docker v0.8 #87

Merged
merged 4 commits into from
Feb 12, 2014
Merged

Support docker v0.8 #87

merged 4 commits into from
Feb 12, 2014

Conversation

nahiluhmot
Copy link
Contributor

@bfulton @tlunter @adamjt

Not very many code changes were required, but two methods no longer work (due to errors in the Docker server). The first is Container#copy will not raise an error if the specified file does not exist. The second is Container#commit will not properly copy over the run configuration.

@adamjt
Copy link

adamjt commented Feb 12, 2014

What is the impact on us re: the 2 methods that no longer work?

@adamjt
Copy link

adamjt commented Feb 12, 2014

API 1.9 is out - any reason we're not using that instead of 1.8?

@nahiluhmot
Copy link
Contributor Author

@adamjt regarding your first question -- no. "No longer work" may not have been the correct term. They are very specific use-cases that have easy workarounds. The former should never impact us so long as we ensure that we copy the correct files. We never used the second way of running an Image.

Regarding your second question, ask @bfulton.

@adamjt
Copy link

adamjt commented Feb 12, 2014

Ah, ok. So these are Docker bugs that should be resolved (hopefully) on their end?

@nahiluhmot
Copy link
Contributor Author

@adamjt yes.

@adamjt
Copy link

adamjt commented Feb 12, 2014

  • 🐕

@@ -140,7 +140,8 @@ def all(opts = {}, conn = Docker.connection)
def search(query = {}, connection = Docker.connection)
body = connection.get('/images/search', query)
hashes = Docker::Util.parse_json(body) || []
hashes.map { |hash| new(connection, hash['Name']) }
puts hashes
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need the puts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

@nahiluhmot
Copy link
Contributor Author

@tlunter I updated the repo to your suggestions.

@bfulton
Copy link
Contributor

bfulton commented Feb 12, 2014

+1

@nahiluhmot
Copy link
Contributor Author

Merging.

nahiluhmot added a commit that referenced this pull request Feb 12, 2014
@nahiluhmot nahiluhmot merged commit fa3f94d into master Feb 12, 2014
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.

4 participants