Allow Client to use a proxy url #7
Conversation
There was a problem hiding this comment.
@t27duck Thank you for submitting this pull request! Proxy support was one of the features from the twitter gem that I wanted to add here, but you beat me to it and your implementation looks great!
I left a few very minor comments, mostly related to the RBS signature file, but I’m optimistic that we can get this merged and released in version 0.9.0 this week.
Thanks so much for your contribution!
lib/x/client.rb
Outdated
| @@ -31,11 +31,12 @@ def initialize(bearer_token: nil, | |||
| debug_output: nil, | |||
| array_class: ResponseHandler::DEFAULT_ARRAY_CLASS, | |||
| object_class: ResponseHandler::DEFAULT_OBJECT_CLASS, | |||
| max_redirects: RedirectHandler::DEFAULT_MAX_REDIRECTS) | |||
| max_redirects: RedirectHandler::DEFAULT_MAX_REDIRECTS, | |||
| proxy_url: nil) | |||
There was a problem hiding this comment.
Small suggestion: The order is technically irrelevant, but I would move this up to follow write_timeout, so the Connection options are all close together.
sig/x.rbs
Outdated
| @@ -93,10 +93,11 @@ module X | |||
| @http_client: Net::HTTP | |||
|
|
|||
| attr_reader base_uri: URI::Generic | |||
| attr_reader proxy_uri: URI::Generic | nil | |||
There was a problem hiding this comment.
The shorthand/syntactic sugar for this is: URI::Generic?
sig/x.rbs
Outdated
| attr_reader open_timeout : Float | Integer | ||
| attr_reader read_timeout : Float | Integer | ||
| attr_reader write_timeout : Float | Integer | ||
| def initialize: (?base_url: URI::Generic | String, ?open_timeout: Float | Integer, ?read_timeout: Float | Integer, ?write_timeout: Float | Integer, ?debug_output: IO?) -> void | ||
| def initialize: (?base_url: URI::Generic | String, ?open_timeout: Float | Integer, ?read_timeout: Float | Integer, ?write_timeout: Float | Integer, ?debug_output: IO?, ?proxy_url: URI::Generic | String | nil) -> void |
sig/x.rbs
Outdated
| @@ -168,7 +170,7 @@ module X | |||
| @response_handler: ResponseHandler | |||
|
|
|||
| attr_reader base_uri: URI::Generic | |||
| def initialize: (?bearer_token: String?, ?api_key: String?, ?api_key_secret: String?, ?access_token: String?, ?access_token_secret: String?, ?base_url: URI::Generic | String, ?content_type: String, ?user_agent: String, ?open_timeout: Float | Integer, ?read_timeout: Float | Integer, ?write_timeout: Float | Integer, ?debug_output: IO?, ?array_class: Class, ?object_class: Class, ?max_redirects: Integer) -> void | |||
| def initialize: (?bearer_token: String?, ?api_key: String?, ?api_key_secret: String?, ?access_token: String?, ?access_token_secret: String?, ?base_url: URI::Generic | String, ?content_type: String, ?user_agent: String, ?open_timeout: Float | Integer, ?read_timeout: Float | Integer, ?write_timeout: Float | Integer, ?debug_output: IO?, ?array_class: Class, ?object_class: Class, ?max_redirects: Integer, ?proxy_url: URI::Generic | String | nil) -> void | |||
|
PS: Don’t worry about the failing checks. They were caused by an update to the |
This change adds a `proxy_url` option when creating the client. If set, the `Net::HTTP` object will set the appropriate values and use a proxy server for the connection.
t27duck
force-pushed
the
proxy_server_option
branch
from
ad74a98 to
6a2618b
Compare
|
Changes suggestions should be applied now. |
|
Just release version 0.9.0 with proxy support. Thanks again for your patch! |
This change adds a
proxy_urloption when creating the client.If set, the
Net::HTTPobject will set the appropriate values and use a proxy server for the connection.