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

Implement HTTPI::Auth::SSL#ciphers configuration #180

Closed
wants to merge 1 commit into from

Conversation

faucct
Copy link

@faucct faucct commented Dec 14, 2016

http-net-persistent only supports this configuration option since 3.0.0, earlier versions will fail with undefined method error, but only if you will use the option.
It also has HTTP::Net::Persistent#initialize signature changed, so I have to support both: old and new.

@rogerleite
Copy link
Member

This request option allows to Sets the available symmetric algorithms for encryption and decryption.

Before merging, I'd like to understand better.

  • In what scenario do you use this option?
  • How to set a "custom" specific symmetric algorithm?

Thanks to make available on all adapters [:httpclient, :curb, :excon, :http, :net_http, :net_http_persistent] with "auth" support.

Regards,

Roger Leite

@rogerleite
Copy link
Member

I'm looking at Travis, I saw that net_http_persistent doesn't support ruby 2.0. We can do something at Gemfile to make Travis green again? https://travis-ci.org/savonrb/httpi/jobs/184017813

@faucct
Copy link
Author

faucct commented Dec 15, 2016

Yes, I can expose the net_http_persistent version into a environmental variable and to use different ones for build. Easier option would be to remove the constraint at all.

Gemfile Outdated
@@ -9,7 +9,7 @@ gem 'curb', '~> 0.8', :require => false, :platforms => :ruby
gem 'em-http-request', :require => false, :platforms => [:ruby, :jruby]
gem 'em-synchrony', :require => false, :platforms => [:ruby, :jruby]
gem 'excon', '~> 0.21', :require => false, :platforms => [:ruby, :jruby]
gem 'net-http-persistent', '~> 2.8', :require => false
gem 'net-http-persistent', '~> 3.0', :require => false
Copy link
Member

Choose a reason for hiding this comment

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

Maybe something like:

if RUBY_VERSION == "2.0.0"
  gem 'net-http-persistent', '~> 2.8',    :require => false
else
  gem 'net-http-persistent', '~> 3.0',    :require => false
end

@anyakhvost
Copy link

This would be awesome! When do you think you will be merging this? @faucct

@faucct
Copy link
Author

faucct commented Feb 8, 2017

I guess I will have time this evening.

@anyakhvost
Copy link

Awesome! Thanks @faucct !

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 74.26% when pulling 3b6d968 on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

1 similar comment
@coveralls
Copy link

coveralls commented Feb 8, 2017

Coverage Status

Coverage decreased (-0.2%) to 74.26% when pulling 3b6d968 on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 74.26% when pulling 865475c on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 74.26% when pulling 865475c on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 74.26% when pulling 865475c on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

@coveralls
Copy link

coveralls commented Feb 8, 2017

Coverage Status

Coverage decreased (-0.2%) to 74.26% when pulling 865475c on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

@coveralls
Copy link

coveralls commented Feb 8, 2017

Coverage Status

Coverage decreased (-0.08%) to 74.374% when pulling 06b6e07 on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

@coveralls
Copy link

coveralls commented Feb 8, 2017

Coverage Status

Coverage decreased (-0.08%) to 74.374% when pulling 61f3f25 on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

@coveralls
Copy link

coveralls commented Feb 8, 2017

Coverage Status

Coverage decreased (-0.08%) to 74.374% when pulling 6d68d09 on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+22.6%) to 97.039% when pulling 6d68d09 on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

@coveralls
Copy link

coveralls commented Feb 8, 2017

Coverage Status

Coverage decreased (-0.08%) to 74.374% when pulling 6d68d09 on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

@faucct
Copy link
Author

faucct commented Feb 8, 2017

Why does the bundle grab the newest version though it is requires newer version of ruby? This is a mistery why all those constraints are needed.

@coveralls
Copy link

coveralls commented Feb 8, 2017

Coverage Status

Coverage decreased (-0.08%) to 74.374% when pulling af7dbff on faucct:feature/open_ssl_ciphers into e671b7a on savonrb:master.

@anyakhvost
Copy link

@faucct Is there anything I could do to help?

@faucct
Copy link
Author

faucct commented Feb 10, 2017

Hello. You can determine what triggers those "jumps" of coverage.
Also I don't like the constraints I've put in Gemfile: they are probably not allowing to test all features. You can have different constraints for different ruby versions using appraisal gem.
Both would really help.

@anyakhvost
Copy link

Hello @faucct, it shows that the coverage decreased for /lib/httpi/auth/ssl.rb. It shows ciphers method is not covered: https://coveralls.io/builds/10060066/source?filename=lib%2Fhttpi%2Fauth%2Fssl.rb However, I do see the spec you wrote for it: https://github.com/savonrb/httpi/pull/180/files#diff-010b812ab47309d70ecf654d470cd331R161

@faucct
Copy link
Author

faucct commented Feb 13, 2017

I was talking about those coverage jumps:
2017-02-13 20 11 36

@anyakhvost
Copy link

HI @faucct, regarding the constrains in Gemfile and testing against different Ruby versions, Travis can test applications against different Ruby versions. In .travis.yml, you can add different versions of Ruby. Would that solve the problem you are referring to?

@faucct
Copy link
Author

faucct commented Feb 14, 2017

The gem is already being tested against different Ruby versions as you can see inside .travis.yml. Now we need to test against different gem dependencies versions.

@rogerleite rogerleite closed this Jul 7, 2020
@c960657
Copy link
Contributor

c960657 commented Nov 5, 2020

Why was this never merged? Seems like a very useful feature.

@rogerleite
Copy link
Member

I don't remember, I closed this issue this year because of inactivity since 2017.
One can try to apply these commits and open a new pull request.

@olleolleolle
Copy link
Contributor

Fixed in #214

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants