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

Replace RestClient gem with HTTP #60

Merged
merged 1 commit into from
Aug 24, 2019
Merged

Conversation

abotalov
Copy link
Member

Here are the reasons for replacing gem:

  • RestClient produces a warning when sending multipart requests with this gem's monkeypatch
  • Monkeypatching should be avoided
  • HTTP gem supports sending needed multipart requests without monkeypatch

@abotalov abotalov requested a review from alalayla August 12, 2019 08:46
@abotalov abotalov self-assigned this Aug 12, 2019
@abotalov abotalov force-pushed the replace-rest-client-with-http branch from 01ae9ee to a4ed59d Compare August 12, 2019 18:55
@abotalov abotalov mentioned this pull request Aug 12, 2019
@abotalov abotalov force-pushed the replace-rest-client-with-http branch from a4ed59d to f94e004 Compare August 12, 2019 18:58
lib/reportportal.rb Show resolved Hide resolved
response
else
p "ReportPortal API returned #{response}"
p "Offending request method/URL: #{request.args[:method].upcase} #{request.args[:url]}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does http handle errors in the same way?

Copy link
Member Author

@abotalov abotalov Aug 13, 2019

Choose a reason for hiding this comment

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

HTTP handles errors in a bit different way. If there is no response from server (connection error, SSL error, DNS error) an error is raised. If there is a response, its code can be inspected.

@vveliev-tc
Copy link

#59 now reuses the connection

@abotalov abotalov force-pushed the replace-rest-client-with-http branch from f38e132 to c61d142 Compare August 24, 2019 13:16
@abotalov abotalov merged commit e002c08 into master Aug 24, 2019
@abotalov abotalov deleted the replace-rest-client-with-http branch August 24, 2019 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants