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

Unix socket support #187

Merged
merged 2 commits into from
Oct 27, 2016
Merged

Unix socket support #187

merged 2 commits into from
Oct 27, 2016

Conversation

rhruiz
Copy link
Contributor

@rhruiz rhruiz commented Oct 26, 2016

Allows connecting to http servers listening on unix sockets

To perform a get /path on /var/run/app.sock (note the required escaping on slashes):

iex> HTTPoison.get! "http+unix://%2Fvar%2Frun%2Fapp.sock/path"

NOTE requires erlang >= 19 and hackney >= 1.6.2

Closes #185

@rhruiz
Copy link
Contributor Author

rhruiz commented Oct 26, 2016

We should probably rerun this after a httparrot update

@@ -1,5 +1,9 @@
# Changelog

# Unreleased

* Add support for http over unix sockets - requires hackey >= 1.6.2, erlang >= 19. (#185).
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a typo: hackeyhackney.
Also you probably should bump the hackney's version to 1.6.2 in mix.exs if this is the version in which the feature was implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issues fixed. Thanks, @nscyclone :)

@rhruiz rhruiz force-pushed the unix_socket_support branch from d99e6c6 to 570d24b Compare October 26, 2016 21:09
@rhruiz
Copy link
Contributor Author

rhruiz commented Oct 26, 2016

And the hackney 1.6.2 package is broken in hex.pm => benoitc/hackney#354

@rhruiz rhruiz force-pushed the unix_socket_support branch 3 times, most recently from 00d57c0 to 179d367 Compare October 27, 2016 10:21
@rhruiz
Copy link
Contributor Author

rhruiz commented Oct 27, 2016

Now this looks good to go :)

@@ -22,9 +22,9 @@ defmodule HTTPoison.Mixfile do

defp deps do
[
{:hackney, "~> 1.6.0"},
{:hackney, "~> 1.6.0 and >= 1.6.3"},
Copy link
Owner

Choose a reason for hiding this comment

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

We can probably just do "~> 1.6.3" and it will do the same (I guess?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

~> 1.6.3 would not update to 1.7.0 when it comes out. I was trying to replicate the old ~> 1.6 requirement

Copy link
Owner

Choose a reason for hiding this comment

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

I see. Got it 👌

Copy link
Owner

Choose a reason for hiding this comment

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

But actually we always(or almost) had ~> MAJOR.MINOR.BUGFIX set. Maybe we should keep it as it is? so just ~> 1.6.3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

@@ -94,6 +94,17 @@ defmodule HTTPoisonTest do
assert_response HTTPoison.get("https://localhost:8433/get", [], ssl: [cacertfile: cacert_file, keyfile: key_file, certfile: cert_file])
end

test "http+unix scheme" do
if Application.get_env(:httparrot, :unix_socket, false) do
Copy link
Owner

Choose a reason for hiding this comment

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

So by default it's false and AFAIK we don't set this to true. How do we know this test is running?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just like your ssl support I've set the default to true in httparrot. It will be false for versions of httparrot that do not support unix sockets yet.

Copy link
Owner

Choose a reason for hiding this comment

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

@edgurgel
Copy link
Owner

Thanks a lot for pushing this forward. Awesome addition to the library 👍

@rhruiz rhruiz force-pushed the unix_socket_support branch from 179d367 to 59654fc Compare October 27, 2016 22:56
@edgurgel
Copy link
Owner

Awesome! Thanks! I will release a new version soon! 👍

@edgurgel edgurgel merged commit 6d01745 into edgurgel:master Oct 27, 2016
@rhruiz rhruiz deleted the unix_socket_support branch October 28, 2016 00:41
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