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

Create CLI option to bind to UNIX socket #265

Merged
merged 1 commit into from
May 12, 2019
Merged

Conversation

Xowap
Copy link
Contributor

@Xowap Xowap commented Apr 12, 2019

As suggested in #264, this PR brings the ability for tusd to bind to a UNIX socket.

The implementation adds a -unix-sock CLI option which if set to a non-empty value will bind tusd to the provided path and discard the TCP host and port.

I'm unsure about the standards of the project, so please me how I can improve this.

@Acconut
Copy link
Member

Acconut commented Apr 17, 2019

Thank you for this PR, it looks very good, all in all. I will try it on my own in the next few days and come back if any problems occur. In the meantime, do you know how this will behave on Windows (or other platforms not supporting UNIX sockets)?

@Xowap
Copy link
Contributor Author

Xowap commented Apr 17, 2019

Good!

Regarding the Windows support, that's an excellent question, I suppose that an error will be raised by the go standard library. I've found this issue which suggests it's going to fail yet it's going to be implemented one day. As it's specific to the implementation of Go, I'd suggest to keep the option available in Windows so that if the code is compiled on a compatible implementation then it automatically works. Though it might get confusing, maybe do that and document it in the help message?

In any case, I'll have to test it.

Copy link
Member

@Acconut Acconut left a comment

Choose a reason for hiding this comment

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

Apologies for my delay. I didn't know that Go on Windows recently gained support for UNIX sockets. In that case, we should just let Go take care of testing support.

@Acconut Acconut merged commit 9667110 into tus:master May 12, 2019
@Xowap
Copy link
Contributor Author

Xowap commented May 13, 2019

Awesome 👍

@Acconut
Copy link
Member

Acconut commented May 13, 2019

I am currently working on a few smaller improvement to tusd, so the UNIX socket support will be shipped in the new release in a few days. And thank you again for the contribution

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