Conversation
…e control during on publish
| if puback_taken: | ||
| raise RuntimeError( | ||
| "acquire_publish_acknowledgement_control() may only be called once per received PUBLISH.") | ||
| if not callback_active: |
There was a problem hiding this comment.
does this need nonlocal declaration too?
|
|
||
| if puback_control_id != 0: | ||
| def acquire_publish_acknowledgement_control(): | ||
| nonlocal puback_taken |
There was a problem hiding this comment.
do we need synchronization support to be thread-safe here? Even though we're in the client's callback, it's still possible for this value to cross thread boundaries and this to be invoked concurrently.
There was a problem hiding this comment.
The Python GIL is held via aws_py_gilstate_ensure() in native C until we call PyGILState_Released() at the end of the s_on_publish_received() callback. The GIL is the lock and keeps everything thread-safe.
| return | ||
| self._on_publish_cb(publish_data) | ||
|
|
||
| callback_active = False |
There was a problem hiding this comment.
I thought we were going to call acquire_publish_acknowledgement_control here to prevent calls to it from succeeding after we exit the callback?
There was a problem hiding this comment.
This feels cleaner than unnecessarily creating a PublishAcknowledgementControlHandle and then storing and returning a separate bool (since we'd be flipping the puback_taken by calling acquire_publish_acknowledgement_control) indicating whether the user took control or we took control. This also allows us to provide a more useful RuntimeError message about trying to call outside of the callback.
|
|
||
| /* If _on_publish returned False/None, the user did not take control of the publish acknowledgement. | ||
| * Auto-invoke it now. */ | ||
| if (puback_control_id != 0 && !PyObject_IsTrue(result)) { |
There was a problem hiding this comment.
can't you combine these two (remove the top one) by changing this to
if (puback_control_id != 0 && (!result || !PyObject_IsTrue(result)) {
...
}
There was a problem hiding this comment.
The top !result is checking for a returned NULL from the call to Python's on_publish method. If that fails we should call PyErr_WriteUnraisable() as well as clearing the control_id whether it was taken or not. In the case of that kind of NULL it's almost certainly not been taken.
2 participants
Bind manual puback
Opaque manual puback control handle
Tests for Manual Puback
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.