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

fixes OpenSSL error issue in ruby 1.9.3 #110

Merged
merged 1 commit into from Apr 9, 2015

Conversation

dustinleblanc
Copy link
Contributor

Closes #103

@bodrovis
Copy link

That's interesting because I was getting the same error with Ruby 2.

@dustinleblanc
Copy link
Contributor Author

@bodrovis was the error intermittent? what version of 2.x were you using?

@bodrovis
Copy link

bodrovis commented Feb 5, 2015

@dustinleblanc It was constant until I double-checked enabled services. I used Ruby 2.0.

if defined?(OpenSSL::SSL::SSLErrorWaitReadable)
errors << OpenSSL::SSL::SSLErrorWaitReadable
end
errors
end
Copy link
Contributor

Choose a reason for hiding this comment

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

@bodrovis I'm sorry for not accepting this PR before, I think this is the correct way.

However, do you mind changing the condition to if RUBY_VERSION > '2'? That's really the implicit requirement that we are making and that we are confirming in the tests. So I would like the method above to be:

def server_errors
  [
    OpenSSL::SSL::SSLError,
    Errno::ETIMEDOUT,
    Errno::ENETUNREACH,
    Errno::ECONNRESET,
    Net::HTTPServerError
  ] + (RUBY_VERSION < '2' ? [] : [OpenSSL::SSL::SSLErrorWaitReadable])
end

If you agree and change your commit to that, I will merge right away 👯

Copy link

Choose a reason for hiding this comment

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

@claudiofullscreen Yeah that looks better however this is not my PR :)
@dustinleblanc

Copy link
Contributor

Choose a reason for hiding this comment

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

Oooops 😹

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making change and re-running specs in 1.9 + 2.0 just to confirm nothing goes boom

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

On Thu, Apr 9, 2015 at 9:40 AM, Dustin LeBlanc notifications@github.com
wrote:

In lib/yt/request.rb
#110 (comment):

     Errno::ETIMEDOUT,
     Errno::ENETUNREACH,
     Errno::ECONNRESET,
     Net::HTTPServerError
   ]
  •  # OpenSSL::SSL::SSLErrorWaitReadable does not exist in Ruby 1.9.3
    
  •  if defined?(OpenSSL::SSL::SSLErrorWaitReadable)
    
  •    errors << OpenSSL::SSL::SSLErrorWaitReadable
    
  •  end
    
  •  errors
    
    end

Making change and re-running specs in 1.9 + 2.0 just to confirm nothing
goes boom


Reply to this email directly or view it on GitHub
https://github.com/Fullscreen/yt/pull/110/files#r28078830.

Claudio Baccigalupo | Software Writer | Fullscreen, Inc.
http://Fullscreen.net | +1.310.202.3333

@claudiofullscreen
Copy link
Contributor

And… can you squash your two commits into one? Thanks again!

@dustinleblanc
Copy link
Contributor Author

NP, clearly my git-fu is getting rusty...too many mend commit messages are showing up lately :)

@dustinleblanc
Copy link
Contributor Author

btw confirmed specs passed in 1.9.3 and 2.2.1

claudiofullscreen pushed a commit that referenced this pull request Apr 9, 2015
fixes OpenSSL error issue in ruby 1.9.3
@claudiofullscreen claudiofullscreen merged commit 95f1c47 into nullscreen:master Apr 9, 2015
@claudiofullscreen
Copy link
Contributor

🎀

@dustinleblanc
Copy link
Contributor Author

👍

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.

None yet

3 participants