Remove default port from grpc ChannelOptions

[shsms]

URIs should always specify a port. If they don't, a ValueError will be raised.

Closes #80

[shsms]

Draft because this is based on #79

Wait wait wait

[llucax]

Mmm, I'm not convinced about completely removing the possibility to set a default port. I agree that for the base client it doesn't make sense, as we don ´t know anything about the API. But for subclasses, where we will probably have well defined service locations, it might make sense to have a default port and only leave a floating address (I can see the case for that if you want to change between staging and production for example).

At least I think @Marenz made a point about this.

I don ´t know, for now I would make the port int | None = None and only raise if the default_port is None.

[shsms]

But for subclasses, where we will probably have well defined service locations, it might make sense to have a default port and only leave a floating address

People can type the ports themselves. We are only introducing opportunities to fail, by providing defaults.

(I can see the case for that if you want to change between staging and production for example).

This example sounds particularly dangerous to me, relying on defaults for switching between staging and production is a definite way to mess things up.

Saving a few keystrokes to type the ports is not worth the risks/complications having a default would introduce.

[Marenz]

It's not about saving keystrokes, it's about making the usage easier.
Optimally, the end user only needs to know dispatch.frequenz.io (or whatever url we use) and the client fills in port/encryption etc automatically.

For staging, of course a different domain would be better dispatch.staging.frequenz.io for example) but I am working with what I got and that's just one server/ip/domain currently ;)

[llucax]

Yeah, I'm even thinking about what if we want to have different versions of an API running in different ports. A service (client) could known which port uses each version, and try to find first the latest version and only connect to older versions if unavailable. I can really see use cases for letting the client chose the port (i.e. using a default). I agree is not about saving keystrokes, is about abstraction.

[llucax]

This example sounds particularly dangerous to me, relying on defaults for switching between staging and production is a definite way to mess things up.

How is it dangerous? Is not relying on defaults, is using different host names, user-provided host names, is using the DNS to look for a particular instance.

[llucax]

There is a reason why port numbers are standarized, the whole IANA thing. Ports do have meaning, even if you could use a non-standard one.

https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml

[shsms]

Fine, let me get out of the way then.

[llucax]

I still see value in part of the PR, I don't think the base client should set a default port, it should just allow clients to set one. If some concrete client think it is best not to provide a default port, they can do so, and that will actually be the default.

I don't know why the door slam 😟

[llucax]

This is what I mean:

• Don't allow a default port in URLs by default #85

[shsms]

I don't think the base client should set a default port, it should just allow clients to set one.

To me it is always better to be explicit, users are going to copy-paste the urls anyway, I don't see how this is useful, and but I think it could mess things up if multiple services are running on the same domain name, like for different users, etc., which we already have examples for. But just opinions, and not really strong ones.

I don't know why the door slam 😟

No, of course not, more like a door opening. It looks like the majority thinks it is nice to have, so I won't block you, but I'd rather not do it myself.

[llucax]

To me it is always better to be explicit, users are going to copy-paste the urls anyway, I don't see how this is useful, and but I think it could mess things up if multiple services are running on the same domain name, like for different users, etc., which we already have examples for.

I agree with all of this, but still in general URLs allow omitting the port, and this is still a general purpose library (even when it might be a bit specific to our use case). With the PR I'm proposing, the default is still to require an explicit port, but if some client thinks a default port makes sense, they can do so. So unless some downstream project explicitly defines a default port, it will be required. If a downstream project goes to the trouble of setting a default, I would trust they know what they are doing. For sure it exist cases for using default ports, they are used everywhere (most services we use on a daily basis have a default port) and they are not required by the URL/URI standard. So I think it makes sense to leave the door open in this library, but go with a safe default.

No, of course not, more like a door opening. It looks like the majority thinks it is nice to have, so I won't block you, but I'd rather not do it myself.

👍