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

Add unix domain socket listener to service.ListenAndServeUnix #837

Merged
merged 4 commits into from
Oct 24, 2016

Conversation

hatajoe
Copy link

@hatajoe hatajoe commented Oct 24, 2016

Hi.

I very often use unix domain socket listener with HTTP proxy server. (e.g, Nginx)
In goa, it looks no way to use that.
This p-r add function service.ListenAndServeUnix that support unix domain socket listener.
Can you review this?

Thank you.

@raphael
Copy link
Member

raphael commented Oct 24, 2016

Thanks for the PR! goa has to compile / run on Windows and in App Engine (package syscall cannot be used there). Also it seems a bit strange to handle signals only when listening on a Unix socket. Given all this and how it's simple to do this in your own code I'm not sure this is worth adding to goa. Is there a reason you cannot have that code outside? Maybe this should be an example instead.

@hatajoe
Copy link
Author

hatajoe commented Oct 24, 2016

Sorry, I was not able to consider multiple environments.
And certainly seems little bit strange.

I will take you another approach that inject listener to function like net.http.Serve.
This approach seems simple than before.
Would you like about this? (I will push soon)

@raphael
Copy link
Member

raphael commented Oct 24, 2016

This approach seems much better indeed, thank you! Just one nitpick: do you think you could change the comment of the method to something like:

// Serve accepts incoming HTTP connections on the listener l, invoking the service mux handler for each.

Thank you!

@hatajoe
Copy link
Author

hatajoe commented Oct 24, 2016

I fixed it.
My English is not good enough ;)
Thank you for your review.

@raphael
Copy link
Member

raphael commented Oct 24, 2016

This is great thank you! Do you think you could also make a PR against the v1 branch?

@raphael raphael merged commit 49636fc into goadesign:master Oct 24, 2016
@hatajoe
Copy link
Author

hatajoe commented Oct 24, 2016

I made a p-r #838 .
but seems something I was wrong?

hatajoe added a commit to hatajoe/goa that referenced this pull request Oct 25, 2016
…ign#837)

* Add ListenAndServeUnix

* Revert "Add ListenAndServeUnix"

This reverts commit 33f8e6e.

* Add Serve function for any listner can start

* Fix comment
raphael pushed a commit that referenced this pull request Oct 25, 2016
…839)

* Add ListenAndServeUnix

* Revert "Add ListenAndServeUnix"

This reverts commit 33f8e6e.

* Add Serve function for any listner can start

* Fix comment
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.

2 participants