You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2022/02/01 17:44:00 UTC
[jira] [Commented] (PROTON-2060) [docs] The documentation for the sender auto-settle option needs clarification
[ https://issues.apache.org/jira/browse/PROTON-2060?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17485390#comment-17485390 ]
ASF GitHub Bot commented on PROTON-2060:
----------------------------------------
astitcher commented on a change in pull request #354:
URL: https://github.com/apache/qpid-proton/pull/354#discussion_r796829418
##########
File path: python/proton/_handlers.py
##########
@@ -663,11 +667,19 @@ class MessagingHandler(Handler, Acking):
simpler to deal with and/or avoids repetitive tasks for common use
cases.
+ .. note:: Sender **auto-settlement** only occurs for a delivery after the
+ sender receives a settled disposition for that delivery. Otherwise,
+ there would be no way to receive any further events for that delivery
+ (such as the subsequent on_settle message that might be expected when
+ the receiver finally settles the message).
+
Review comment:
Putting this comment here seems unmotivated. It's just not connected narratively with the comment before.
##########
File path: python/proton/_handlers.py
##########
@@ -950,11 +962,19 @@ class TransactionalClientHandler(MessagingHandler, TransactionHandler):
and provides a convenience method :meth:`accept` for performing
a transactional acceptance of received messages.
+ .. note:: Sender **auto-settlement** only occurs for a delivery after the
+ sender receives a settled disposition for that delivery. Otherwise,
+ there would be no way to receive any further events for that delivery
+ (such as the subsequent on_settle message that might be expected when
+ the receiver finally settles the message).
+
:param prefetch: Initial flow credit for receiving messages, defaults to 10.
- :param auto_accept: If ``True``, accept all messages (default). Otherwise messages
- must be individually accepted or rejected.
+ :param auto_accept: If ``True``, accept all messages (default). Otherwise
+ messages must be individually accepted or rejected.
:param auto_settle: If ``True``, settle all messages (default). Otherwise
- messages must be explicitly settled.
+ messages must be explicitly settled. Sender auto-settlement only occurs
+ for a delivery after the sender receives a settled disposition for that
+ delivery.
Review comment:
Again use the previous suggestion.
##########
File path: python/proton/_handlers.py
##########
@@ -50,6 +50,10 @@ class OutgoingMessageHandler(Handler):
:param auto_settle: If ``True``, settle all messages (default). Otherwise
messages must be explicitly settled.
+
+ .. note:: Sender auto-settlement only occurs for a delivery after the
Review comment:
Delete this as a separate para add the comment into the basic description of auto settle behaviour as in:
If ``True`` (default), automatically settle messages upon receiving a settled disposition for that delivery...
##########
File path: cpp/include/proton/sender_options.hpp
##########
@@ -82,7 +82,16 @@ class sender_options {
/// Set the delivery mode on the sender.
PN_CPP_EXTERN sender_options& delivery_mode(delivery_mode);
- /// Automatically settle messages (default is true).
+ /**
+ * Automatically settle messages (default is true).
+ *
+ * \note
+ * Sender **auto-settlement** only occurs for a delivery after the
+ * sender receives a settled disposition for that delivery. Otherwise,
+ * there would be no way to receive any further events for that delivery
+ * (such as the subsequent on_settle message that might be expected when
+ * the receiver finally settles the message).
+ */
Review comment:
I think the suggested wording I made for the python API is good here too (see below). I don't see the need to call out auto-settlement in so many words, just explain the basic behaviour as below.
In the API docs that explain settlement we should explain that you can receive no more events after settling a delivery, but that understanding is fundamental to what *settlement is* and so shouldn't need explaining every place we talk about settlement.
##########
File path: ruby/lib/core/sender.rb
##########
@@ -29,10 +29,17 @@ class Sender < Link
# Open the {Sender} link
#
+ # @note Sender **auto-settlement** only occurs for a delivery after the
+ # sender receives a settled disposition for that delivery. Otherwise,
+ # there would be no way to receive any further events for that delivery
+ # (such as the subsequent on_settle message that might be expected when
+ # the receiver finally settles the message).
Review comment:
I honestly don't understand why auto-settlement is called out specially here (or above). There's nothing special about it. Its described below under its option.
##########
File path: c/include/proton/delivery.h
##########
@@ -287,6 +287,12 @@ PN_EXTERN void pn_delivery_abort(pn_delivery_t *delivery);
* @note If pn_delivery_current(delivery) is true before the call then
* pn_link_advance(pn_delivery_link(deliver)) is called automatically.
*
+ * @note The sender **should not** settle after only receiving a terminal
+ * status disposition with no settle flag, as then there would then be no way
+ * to receive any further events for that delivery (such as the subsequent
+ * on_settle message that might be expected when the receiver finally settles
+ * the message).
+ *
Review comment:
What does settlement have to do with delivery abort? If there is a connection it needs to be called out explicitly to explain why this comment is here. Afaict If the sender aborts a delivery then it's likely they actually *do* want to settle the delivery because the delivery is now effectively dead and why would they want any more events about it? If you are making a comment relevant to this point then say that explicitly.
Essentially I think **should not** is way too strong here - if the application has no need for the receive settlement after aborting the message, and it seems to me likely they don't, then settling the delivery at the sender end first should be fine.
##########
File path: ruby/lib/core/sender.rb
##########
@@ -29,10 +29,17 @@ class Sender < Link
# Open the {Sender} link
#
+ # @note Sender **auto-settlement** only occurs for a delivery after the
+ # sender receives a settled disposition for that delivery. Otherwise,
+ # there would be no way to receive any further events for that delivery
+ # (such as the subsequent on_settle message that might be expected when
+ # the receiver finally settles the message).
# @overload open_sender(address)
# @param address [String] address of the target to send to
# @overload open_sender(opts)
- # @option opts [Boolean] :auto_settle (true) if true, automatically settle transfers
+ # @option opts [Boolean] :auto_settle (true) if true, automatically settle transfers.
+ # **Note:** Sender auto-settlement only occurs for a delivery after the sender receives
+ # a settled disposition for that delivery.
Review comment:
As above we're just describing the basic behaviour, do inline as I suggested for python.
##########
File path: python/proton/_handlers.py
##########
@@ -663,11 +667,19 @@ class MessagingHandler(Handler, Acking):
simpler to deal with and/or avoids repetitive tasks for common use
cases.
+ .. note:: Sender **auto-settlement** only occurs for a delivery after the
+ sender receives a settled disposition for that delivery. Otherwise,
+ there would be no way to receive any further events for that delivery
+ (such as the subsequent on_settle message that might be expected when
+ the receiver finally settles the message).
+
:param prefetch: Initial flow credit for receiving messages, defaults to 10.
:param auto_accept: If ``True``, accept all messages (default). Otherwise
messages must be individually accepted or rejected.
:param auto_settle: If ``True``, settle all messages (default). Otherwise
- messages must be explicitly settled.
+ messages must be explicitly settled. Sender auto-settlement only occurs
+ for a delivery after the sender receives a settled disposition for that
+ delivery.
Review comment:
Use the suggested wording as above. We are describing the basic behaviour here and there is no need for a confusing qualification IMO.
##########
File path: python/proton/_handlers.py
##########
@@ -950,11 +962,19 @@ class TransactionalClientHandler(MessagingHandler, TransactionHandler):
and provides a convenience method :meth:`accept` for performing
a transactional acceptance of received messages.
+ .. note:: Sender **auto-settlement** only occurs for a delivery after the
+ sender receives a settled disposition for that delivery. Otherwise,
+ there would be no way to receive any further events for that delivery
+ (such as the subsequent on_settle message that might be expected when
+ the receiver finally settles the message).
+
Review comment:
As above delete this para we cover it below under auto settlement.
If we want to explain this point we need to attach the explanation to the explanation of settlement as a concept,.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
> [docs] The documentation for the sender auto-settle option needs clarification
> ------------------------------------------------------------------------------
>
> Key: PROTON-2060
> URL: https://issues.apache.org/jira/browse/PROTON-2060
> Project: Qpid Proton
> Issue Type: Improvement
> Components: documentation
> Reporter: Andrew Stitcher
> Priority: Major
>
> The current documentation is silent about the details of when the sender automatically settles. It is significant that sender settlement only happens when the sender receives a receiver settle.
> The sender *should not* settle after only receiving a terminal status disposition with no settle flag, as then there would then be no way to receive any further events for that delivery (such as the subsequent on_settle message that might be expected when the receiver finally settles the message).
> So the documentation for each language binding (and for the Proton API itself) should specify that sender auto-settlement only occurs for a delivery after the sender receives a settled disposition for that delivery.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org