-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add support for a variable number of callback arguments #1
Conversation
pigeon/client.py
Outdated
@@ -148,7 +149,14 @@ def _handle_message(self, message_frame: Frame): | |||
if message_frame.headers.get("version") != self._msg_versions.get(topic): | |||
raise exceptions.VersionMismatchException | |||
message_data = self._topics[topic].deserialize(message_frame.body) | |||
self._callbacks[topic](topic, message_data) | |||
args = (message_data, topic, message_frame.headers, self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this approach is overly complicated and it leads to 2 issues.
- Keywords are now order dependent (so why are they keywords?)
- Variable names have to be precise
I think if you want to keep this approach, you would be better off iteratively mapping values to parameter names instead of a comprehension and range. You could then even get a little bit extra and check if any argument name is a key to the message and extract it or something like this.
However, my approach would be to keep at most 2 signatures.
f(topic, message)
f(topic, message, timestamp, io)
Since the header is in the message, I wouldn't extract it.
…ction to robustly provide the correct arguments
I decided not to include a reference to the |
This pull request allows callbacks to be written with between 0 and 4 arguments. Python's
inspect.signature()
method is then used to determine the correct number of arguments to provide to the function. In order, the provided arguments are,