New properties for protected attributes so downstream can depend on them

[skewty]

I am hoping the PAHO team would be open to exposing some currently protected attributes as properties so downstream projects can / will couple to them.

Reference: https://github.com/sbtinstruments/aiomqtt/issues/191

Example of what is desired:

class NewClient(mqtt.Client):
    """Paho client with some exposed protected values"""

    @property
    def client_id(self) -> str:
        return self._client_id.decode()

    @client_id.setter
    def client_id(self, value: str) -> None:
        if value == "":
            if self._protocol == ProtocolVersion.V31:
                self._client_id = base62(uuid4().int, padding=22)  # copied from paho code
            else:
                self._client_id = b""
        else:
            self._client_id = value.encode()

    @property
    def host(self) -> str:
        return self._host

    @host.setter
    def host(self, value: str):
        self._host = str(value)

    @property
    def port(self) -> int:
        return self._port

    @port.setter
    def port(self, value: int) -> None:
        value = int(value)
        if 0x0000 <= value <= 0xFFFF:
            self._port = value
        raise ValueError("port number out of range")

    @property
    def username(self) -> str | None:
        return None if self._username is None else self._username.decode()

    @username.setter
    def username(self, value: str | None):
        self._username = None if value is None else value.encode()

    @property
    def password(self) -> str | None:
        return None if self._password is None else self._password.decode()

    @password.setter
    def password(self, value: str | None):
        self._password = None if value is None else value.encode()

    @property
    def connect_timeout(self) -> float:
        return self._connect_timeout

    @connect_timeout.setter
    def connect_timeout(self, value: float) -> None:
        value = float(value)
        if value <= 0.0:
            raise ValueError("timeout must be a positive number")
        self._connect_timeout = value

    @property
    def keep_alive(self) -> int:
        return self._keepalive

    @keep_alive.setter
    def keep_alive(self, value: int) -> None:
        value = int(value)
        if value <= 0:
            raise ValueError("keepalive must be a positive number")
        self._keepalive = value

    @property
    def max_inflight_messages(self) -> int:
        return self._max_inflight_messages

    @max_inflight_messages.setter
    def max_inflight_messages(self, value: int) -> None:
        if value < 0:
            raise ValueError("value must not be negative")
        self._max_inflight_messages = value

    @property
    def max_queued_messages(self) -> int:
        return self._max_queued_messages

    @max_queued_messages.setter
    def max_queued_messages(self, value: int) -> None:
        if value < 0:
            raise ValueError("value must not be negative")
        self._max_queued_messages = value

    @property
    def will_topic(self) -> str:
        return self._will_topic.decode()

    @property
    def will_payload(self) -> bytes:
        return self._will_payload

[PierreF]

I think that read properties are fine and will be done.

Write properties are an issue to me. What means changing the client_id when you are connected ? Should we disconnect and reconnect with new client_id ? Only wait for next reconnection to use the new ID (which could happen days later) ?

[PierreF]

I've added with write for most properties. Some will only apply on next reconnection (as described in docstring). Some write are unsupported if the connection is established due to possible unexpected condition. In that case it will a warning. Finally some few their is not write because it's pretty sure to broken if the connection is established (e.g. protocol)

[akx]

As discussed in the pull request, I'm not convinced it's necessarily the best idea to add setters for all properties, especially when they won't take effect immediately.

@skewty (and @frederikaalund from the aiomqtt issue): what would be the use case for setting e.g. a host or port for a client that's already connected? Could one not just create a new client in that case?

[skewty]

Creating a new client requires, of course, re-binding all the callbacks as well. If all my code knows about is the new host + port to use that becomes an issue.

My use case is somewhat complicated because it uses asyncio and paho is a hidden requirement. Downstream developers are reluctant to couple to protected fields (as they should be). Small decisions can have larger impacts due to this.

What is likely to happen is other projects just keep their own copy of values that are already in paho. I guess I was hoping paho would make these changes to be a little bit more open for extension (open-closed principle in SOLID) even if it doesn't need the function itself.

[PierreF]

I don't get the use-case for writing host property with aiomqtt. It's API is made such as when the Client (paho-mqtt & aiomqtt) is created, the host is already decided and immutable or did I miss something ?

I believe only two options are valid (and prefer the first):

• either don't allow to modify at all host, port or other options that only apply on future connection. host, port and keepalive are settable before (re)establishing the connection, using connect()
• raise error if trying to set them while connection is open or being currently being opened

If we don't do that, the following would be nondeterministic:

client = Client()
client.connect_async("host1")
client.loop_start()
client.host = "host2"

I think we might be able to write similar case in aiomqtt (we only need to have concurrency between the connection & the client.host = "host2". The result in not deterministic, it could connect to host "host1" or "host2".

It seems much easier to just don't allow writing host, port and keepalive, especially since those 3 value are overwrite by connect() anyway.

[PierreF]

As commented in the PR, I'll disallow changed connection related properties while connected or connecting. It'll only be allowed to change them before first connect/connect_async/reconnect or after disconnect()