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

Allow setting custom options for Faraday, like proxy or SSL certs #106

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

skalee
Copy link
Contributor

@skalee skalee commented Dec 1, 2017

Fixes #105.

Ribose::Request.get("ping")

expect(
a_request(:any, //).with(headers: custom_headers)

Choose a reason for hiding this comment

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

Put a comma after the last parameter of a multiline method call.

custom_headers = {"X-Set-In-Config" => "Yes"}

stub_configuration do |config|
config.faraday_options = {headers: custom_headers}

Choose a reason for hiding this comment

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

Space inside { missing.
Space inside } missing.

@@ -60,6 +60,21 @@
end
end

example "custom Faraday options from configuration are used" do
custom_headers = {"X-Set-In-Config" => "Yes"}

Choose a reason for hiding this comment

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

Space inside { missing.
Space inside } missing.

file_upload = Ribose::FileUploader.upload(space_id, file_attributes)

expect(
a_request(:any, //).with(headers: custom_headers)

Choose a reason for hiding this comment

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

Put a comma after the last parameter of a multiline method call.

end

stub_ribose_space_file_upload_api(space_id, file_attributes)
file_upload = Ribose::FileUploader.upload(space_id, file_attributes)

Choose a reason for hiding this comment

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

Useless assignment to variable - file_upload.

custom_headers = {"X-Set-In-Config" => "Yes"}

stub_configuration do |config|
config.faraday_options = {headers: custom_headers}

Choose a reason for hiding this comment

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

Space inside { missing.
Space inside } missing.

@@ -16,6 +16,22 @@
end
end

example "custom Faraday options from configuration are used" do
space_id = 123_456_789
custom_headers = {"X-Set-In-Config" => "Yes"}

Choose a reason for hiding this comment

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

Space inside { missing.
Space inside } missing.

@@ -9,17 +9,20 @@
api_host = "www.example.com"
api_token = "SUPER_SECRET_API_TOKEN"
user_email = "[email protected]"
faraday_options = {some: :options}

Choose a reason for hiding this comment

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

Space inside { missing.
Space inside } missing.

Ribose::Request.get("ping")

expect(
a_request(:any, //).with(headers: custom_headers)

Choose a reason for hiding this comment

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

Put a comma after the last parameter of a multiline method call.

custom_headers = {"X-Set-In-Config" => "Yes"}

stub_configuration do |config|
config.faraday_options = {headers: custom_headers}

Choose a reason for hiding this comment

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

Space inside { missing.
Space inside } missing.

@@ -60,6 +60,21 @@
end
end

example "custom Faraday options from configuration are used" do
custom_headers = {"X-Set-In-Config" => "Yes"}

Choose a reason for hiding this comment

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

Space inside { missing.
Space inside } missing.

file_upload = Ribose::FileUploader.upload(space_id, file_attributes)

expect(
a_request(:any, //).with(headers: custom_headers)

Choose a reason for hiding this comment

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

Put a comma after the last parameter of a multiline method call.

end

stub_ribose_space_file_upload_api(space_id, file_attributes)
file_upload = Ribose::FileUploader.upload(space_id, file_attributes)

Choose a reason for hiding this comment

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

Useless assignment to variable - file_upload.

custom_headers = {"X-Set-In-Config" => "Yes"}

stub_configuration do |config|
config.faraday_options = {headers: custom_headers}

Choose a reason for hiding this comment

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

Space inside { missing.
Space inside } missing.

@@ -16,6 +16,22 @@
end
end

example "custom Faraday options from configuration are used" do
space_id = 123_456_789
custom_headers = {"X-Set-In-Config" => "Yes"}

Choose a reason for hiding this comment

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

Space inside { missing.
Space inside } missing.

@@ -9,17 +9,20 @@
api_host = "www.example.com"
api_token = "SUPER_SECRET_API_TOKEN"
user_email = "[email protected]"
faraday_options = {some: :options}

Choose a reason for hiding this comment

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

Space inside { missing.
Space inside } missing.

@skalee
Copy link
Contributor Author

skalee commented Dec 1, 2017

Due to reasons outlined in #94 (comment), do not merge before aproval.

Copy link
Member

@abunashir abunashir left a comment

Choose a reason for hiding this comment

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

Thanks @skalee, We want to go as minimal as possible, as we might even consider that disabling ssl support in the future (details on the issue).

So instead of making whole faraday customisable, what do you think about only adding one option something like Ribose.configuration.ssl_verify_mode, and set defaults to true and update client reference?

Edit:

And let's not change anything in faraday config in file uploader, as the upload url is returned from our file upload API and it's aws url, which should not require us to disable SSL.

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.

3 participants