search

upserve / docker-api

A lightweight Ruby client for the Docker Remote API
MIT License
1.09k stars 289 forks source link

Replace deprecated URI.encode with CGI.escape #551

Closed jasonheecs closed 4 years ago

jasonheecs commented 4 years ago

URI.escape and URI.encode are deprecated, this PR replaces the URI.encode method call with CGI.escape

image

clintoncwolfe commented 4 years ago

My team is looking for this as well, as this is very noisy under ruby 2.7.

tas50 commented 4 years ago

@tlunter any chance we could get a release with this fix pushed to Rubygems. This is going to cause a lot of issues for anyone going to Ruby 2.7.

ypal commented 4 years ago

:+1: on this, upgrading to ruby 2.7 made the output VERY noisy

tas50 commented 4 years ago

For anyone running into this our fork here has all the URI -> CGI fixes. There's more than just this one:https://github.com/chef/docker-api

tlunter commented 4 years ago

@jasonheecs can we replace all URI.encode calls rather than just one?

tlunter commented 4 years ago

Actually, it seems like you can't just find and replace since URI.encode won't encode characters like / and : but CGI.escape does. The remaining URI method encode_www_form_component mirrors CGI.escape. The daemons that I tested against on travis seem to not like those characters being encoded like that.

hezanathos commented 4 years ago

Does it ? It looks like travis failed for a different reason. And it succeded for several ruby versions.

ERROR:  Error installing bundler:

    The last version of bundler (>= 0) to support your Ruby & RubyGems was 1.17.3. Try installing it with `gem install bundler -v 1.17.3`

    bundler requires Ruby version >= 2.3.0. The current ruby version is 2.2.0.

The command "gem install bundler" failed and exited with 1 during .
tlunter commented 4 years ago

@hezanathos this was on a branch where I replaced all occurrences and not just the one in this PR and adjusted the .travis.yml file to test relevant versions of ruby instead of the current ones which are out of date.

tlunter commented 4 years ago

Hey folks, I have merged this in and released as 2.0.0.pre.1. Let me know if you see any issues. Thanks @jasonheecs.