Skip to content

Add properties to access Client attributes#794

Merged
PierreF merged 8 commits into
masterfrom
add-properties
Jan 21, 2024
Merged

Add properties to access Client attributes#794
PierreF merged 8 commits into
masterfrom
add-properties

Conversation

@PierreF
Copy link
Copy Markdown
Contributor

@PierreF PierreF commented Jan 6, 2024

Fix #764, add properties for

  • host
  • port
  • keepalive
  • transport
  • protocol
  • connect_timeout
  • username
  • password
  • max_inflight_messages
  • max_queued_messages
  • will_topic
  • will_payload
  • logger

Copy link
Copy Markdown
Contributor

@akx akx left a comment

Choose a reason for hiding this comment

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

Grammatical etc. fixes within – and also, I'm not sure there should be setters at all for the properties which won't take an effect until reconnect? Are there actual use cases for those?

Comment thread src/paho/mqtt/client.py Outdated
Comment thread src/paho/mqtt/client.py Outdated
Comment thread src/paho/mqtt/client.py Outdated
Comment thread src/paho/mqtt/client.py Outdated
Comment thread src/paho/mqtt/client.py Outdated
Comment on lines +666 to +672
# The issue here is that the previous value of keepalive matter to possibly
# sent ping packet.
warnings.warn(
"updating keepalive on established connection is not supported",
stacklevel=2,
)
self._sock.settimeout(value)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this should just raise a RuntimeError if the behavior is undefined?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've used warnings because in some case it will work... but only some. I'll change to error as it will avoid user trying to use this unsupported usage.

Comment thread src/paho/mqtt/client.py Outdated
Comment thread src/paho/mqtt/client.py Outdated
Comment thread src/paho/mqtt/client.py Outdated
if self._will_topic is None:
return None

return self._will_topic.decode("utf-8")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

.decode("utf-8") is at odds with the type signature bytes | None.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's also odd that we use self._will_TOPIC in the will_PAYLOAD property. Copy/paste is not always your friend :)

Comment thread src/paho/mqtt/client.py Outdated
return self._logger

@logger.setter
def logger(self, value: logging.Logger) -> None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can't unset the logger?

Comment thread src/paho/mqtt/client.py Outdated
@PierreF
Copy link
Copy Markdown
Contributor Author

PierreF commented Jan 10, 2024

I've added setter, because some properties can't be changed without setter (at least connection_timeout), and then I've added setter for other for consistency. But I'm also unsure this will be very useful.

@PierreF PierreF force-pushed the add-properties branch 2 times, most recently from b065003 to 1f3a4e9 Compare January 20, 2024 20:35
@PierreF
Copy link
Copy Markdown
Contributor Author

PierreF commented Jan 20, 2024

I'm going with disallowing (raise RuntimeError) changing properties that need to reconnect. It's only allowed to change them when not connected or connecting. It still disallowed if the connection is currently down because a loop_forever or loop_start might reconnect concurrently. It's required to disconnect() which ensure no automatic reconnection occur

PierreF and others added 6 commits January 21, 2024 12:52
Co-authored-by: Aarni Koskela <akx@iki.fi>
It's now possible to known when client is connecting/connected
and disconnecting/disconnected.
The difference between the two state is mostly whether the connection
is opened or not.
This will allow to avoid a reconnection while a reconnection is ongoing.
@PierreF PierreF merged commit 3ec0a98 into master Jan 21, 2024
@PierreF PierreF deleted the add-properties branch January 21, 2024 14:16
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.

New properties for protected attributes so downstream can depend on them

2 participants