allow request's timeout and open timeout to be configurable

okhwaja commented 3 years ago

related to feature request

https://github.com/mapbox/mapbox-sdk-rb/issues/54

okhwaja commented 3 years ago

@samfader - i'm not sure why the test suite didn't run. did I do something incorrectly?

samfader commented 3 years ago

@okhwaja Hmmm. I'm not sure either. Are you able to run them locally?

okhwaja commented 3 years ago

hey @samfader , yes it works locally for me

MapboxAccessToken=xxxxxx bundle exec ruby -Itest test/all_tests.rb
[Coveralls] Set up the SimpleCov formatter.
[Coveralls] Using SimpleCov's default settings.
Loaded suite test/all_tests
Started
...........................
Finished in 8.196894 seconds.
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
27 tests, 34 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
3.29 tests/s, 4.15 assertions/s
[Coveralls] Outside the CI environment, not sending data.

samfader commented 3 years ago

I'm not quite sure why they aren't running on this repo. This repo doesn't really have a maintainer at this point so it's tricky.

@andrii-rymar I know you've contributed somewhat recently here, would you be willing to be a third set of eyes on this PR?

andrii-rymar commented 3 years ago

@okhwaja @samfader added few comments but they are totally optional and it looks good enough to me even in the present shape.

okhwaja commented 3 years ago

hey @andrii-rymar @samfader , made the changes suggested in the comments. Verified the test suite still passes

MapboxAccessToken=xxxxxx bundle exec ruby -Itest test/all_tests.rb
[Coveralls] Set up the SimpleCov formatter.
[Coveralls] Using SimpleCov's default settings.
Loaded suite test/all_tests
Started
............................
Finished in 1.331453 seconds.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
28 tests, 33 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
21.03 tests/s, 24.78 assertions/s

andrii-rymar commented 3 years ago

@okhwaja LGTM

@samfader what do you think?

okhwaja commented 3 years ago

gentle ping @samfader 😅 , just wanna be sure this doesn't fall off your radar

for context - we had an issue a few weeks ago where requests to mapbox were hanging and we're hoping to configure some timeouts to reduce the blast radius on our systems

samfader commented 2 years ago

@okhwaja Here it is: https://rubygems.org/gems/mapbox-sdk/versions/2.3.3

okhwaja commented 2 years ago

thanks @samfader !!

mapbox / mapbox-sdk-rb

allow request's timeout and open timeout to be configurable #55