Changes to server to allow better control over binding to interfaces

[powerjungle]

No description provided.

[Et0h]

Hi, thanks for sharing your work on this.

The proposed changes seem like they might be useful for someone, but I can't recall anyone ever asking for such features.

Have you come across a use case for these changes that justify adding to the complexity of the server code?

[powerjungle]

Have you come across a use case for these changes that justify adding to the complexity of the server code?

Like I said in 0123325, when I start the server with IPv6 disabled in the OS, it outputs an error:

Couldn't listen on :::8999: [Errno 97] Address family not supported by protocol.

This isn't an issue, since it seems to just ignore IPv6 after that, at least on this version, who knows what happens in future versions.

Keeping the setup as minimal as possible helps with avoiding trouble in the future.

Some people have a VPN setup and want to host through it, in that case for privacy/security reasons it makes sense to listen only to the VPN interface.

It doesn't really complicate the code that much and I don't see a reason for not having the options. Lots of applications allow you to do this. It's a small change allowing lots of flexibility.

[daniel-123]

It looks like a good feature to me.

One thing I'd put to consider is whether having options like --ipv4-only and --ipv6-only rather than calling it disabling whichever protocol. Given those options are effectively mutually exclusive they probably should also have some message when somebody tries to use both at the same time. Or maybe just making it a single option that takes IP protocol version as argument.

[powerjungle]

One thing I'd put to consider is whether having options like --ipv4-only and --ipv6-only

Yeah I was considering that initially, but just went with the more "minimal" approach.

probably should also have some message when somebody tries to use both at the same time

Now when I think about it, this way we don't have to rely on the rest of the code to handle no binding to any interface, it's more explicit.

Or maybe just making it a single option that takes IP protocol version as argument.

This seems even better. It can be something like --only-IP-version 6 or --only-IP-version 4. This way the translation can be a single string, instead of duplicating it in every file just to change a number.

I can rebase the commit depending on what you feel would be the best approach, but I like the last one most so far.

[Et0h]

@powerjungle I've discussed the matter with @daniel-123 and @albertosottile and we've decided that using --ipv4-only and --ipv6-only would be the best approach. If you could re-base your PR on this basis then that would be great.

[powerjungle]

One issue that I found is, when setting a single invalid IP using --interface-ipv4 or --interface-ipv6 when listening on both versions, it will continue without printing anything. The way to force it to print is by forcing this if statement to False:

syncplay/syncplay/ep_server.py

Line 52 in e1bcf12

if ServerStatus.listening6 or ServerStatus.listening4:

It will not report that it wasn't able to listen to IPv6 when it's disabled system-wide as well.

[Et0h]

Thanks for the heads up on those potential issues, which should be helpful for those wanting to track down issues if they occur in the future. I do not consider those to be blocking problems and so will be accepting this PR.