Thanks for the update. I've put some specific comments in line, but overall
this should be a big improvement.

--Rafael

On Thu, Oct 24, 2013 at 1:33 PM, Michael Goulish <mgoul...@redhat.com>wrote:

>
>
>   Dear Proton Proponents --
>
>
>     Here is my proposed text for Python Messenger API documentation.
>
>     If you'd like to comment, please do so within the next week.
>     I will incorporate feedback and check in the resulting
>     changes to the codebase at the stroke of midnight, on
>     All Hallows Eve.  ( Samhain. )
>
>
>     I have given you the current text for each method and property,
>     and then my changes.  My changes are either proposed replacements
>     ( NEW_TEXT ) or proposed additions ( ADD_TEXT ).
>
>     Mostly, this is highly similar to the C API text, but with
>     minor changes for Pythonification.
>
>
>   ------------------------------ Mick .
>
>
>
>
>     Class Comments
>     {
>       CURRENT_TEXT
>       {
>         The Messenger class defines a high level interface for
>         sending and receiving Messages. Every Messenger contains
>         a single logical queue of incoming messages and a single
>         logical queue of outgoing messages. These messages in these
>         queues may be destined for, or originate from, a variety of
>         addresses.
>       }
>
>       ADD_TEXT
>       {
>         The messenger interface is single-threaded.  All methods
>         except one ( interrupt ) are intended to be used from within
>         the messenger thread.
>       }
>     }
>
>
>
>
>     Sending & Receiving Messages
>     {
>       CURRENT_TEXT
>       {
>         The L{Messenger} class works in conjuction with the L{Message}
>         class. The L{Message} class is a mutable holder of message content.
>         The L{put} method will encode the content in a given L{Message}
>         object into the outgoing message queue leaving that L{Message}
>         object free to be modified or discarded without having any impact
> on
>         the content in the outgoing queue.
>
>         Similarly, the L{get} method will decode the content in the
> incoming
>         message queue into the supplied L{Message} object.
>       }
>
>
>
>       NEW_TEXT
>       {
>         The Messenger class works in conjuction with the Message class. The
>         Message class is a mutable holder of message content.
>
>         The put method copies its message to the outgoing queue, and may
>         send queued messages if it can do so without blocking.  The send
>         method blocks until it has sent the requested number of messages,
>         or until a timeout interrupts the attempt.
>
>         Similarly, the recv() method receives messages into the incoming
>         queue, and may block until it has received the requested number of
>         messages, or until timeout is reached.  The get method pops the
>         eldest message off the incoming queue and copies it into the
> message
>         object that you supply.  It will not block.
>       }
>

The blocking behavior of recv is actually slightly different than what you
state. It will block until it recvs *up to* N messages.

It's also probably worth noting somewhere that the blocking flag turns off
blocking entirely for the whole API with the exception
Messenger.work()/pn_messenger_work(). With blocking turned off calls like
send/recv will do what work they can without blocking and then return. You
can then use interrogatives like checking the number if incoming/outgoing
messages to see what outstanding work still remains.


>
>
>       NOTE
>       {
>         I thought it would be better in this comment to only emphasize
>         the blocking and non-blocking differences between get/put and
>         recv/send.  Details about how the arg message is handled are moved
>         to the comments for specific methods.
>       }
>
>     }
>
>
>
>
>     Method Details
>     {
>       __init__
>       {
>         CURRENT_TEXT
>         {
>           Construct a new L{Messenger} with the given name. The name has
>           global scope. If a NULL name is supplied, a L{uuid.UUID} based
>           name will be chosen.
>         }
>
>         NEW_TEXT
>         {
>           // no change
>         }
>       }
>
>
>       __del__
>       {
>         CURRENT_TEXT
>         {
>           // none
>         }
>
>         NEW_TEXT
>         {
>           Destroy the messenger.  This will close all connections that
>           are managed by the messenger.  Call the stop method before
>           destroying the messenger.
>         }
>       }
>
>
>       start
>       {
>         CURRENT_TEXT
>         {
>           Transitions the L{Messenger} to an active state. A L{Messenger}
> is
>           initially created in an inactive state. When inactive a
>           L{Messenger} will not send or receive messages from its internal
>           queues. A L{Messenger} must be started before calling L{send} or
>           L{recv}.
>         }
>
>         NEW_TEXT
>         {
>           Currently a no-op placeholder.
>           For future compatibility, do not send or receive messages
>           before starting the messenger.
>         }
>       }
>
>
>       stop
>       {
>         CURRENT_TEXT
>         {
>           Transitions the L{Messenger} to an inactive state. An inactive
>           L{Messenger} will not send or receive messages from its internal
>           queues. A L{Messenger} should be stopped before being discarded
> to
>           ensure a clean shutdown handshake occurs on any internally
> managed
>           connections.
>         }
>
>         NEW_TEXT
>         {
>           // no change
>         }
>       }
>
>
>       subscribe
>       {
>         CURRENT_TEXT
>         {
>           Subscribes the L{Messenger} to messages originating from the
>           specified source. The source is an address as specified in the
>           L{Messenger} introduction with the following addition. If the
>           domain portion of the address begins with the '~' character, the
>           L{Messenger} will interpret the domain as host/port, bind to it,
>           and listen for incoming messages. For example "~0.0.0.0",
>           "amqp://~0.0.0.0", and "amqps://~0.0.0.0" will all bind to any
>           local interface and listen for incoming messages with the last
>           variant only permitting incoming SSL connections.
>         }
>

Does this mean there is no change for subscribe?


>       }
>
>
>       put
>       {
>         CURRENT_TEXT
>         {
>           Places the content contained in the message onto the outgoing
>           queue of the L{Messenger}. This method will never block, however
>           it will send any unblocked L{Messages<Message>} in the outgoing
>           queue immediately and leave any blocked L{Messages<Message>}
>           remaining in the outgoing queue. The L{send} call may be used to
>           block until the outgoing queue is empty. The L{outgoing} property
>           may be used to check the depth of the outgoing queue.
>         }
>
>         ADD_TEXT
>         {
>           When the content in a given Message object is copied to the
> outgoing
>           message queue, you may then modify or discard the message object
>           without having any impact on the content in the outgoing queue.
>         }
>

It is probably worth mentioning that put/get return trackers. Note that
this is different from the C API where the return value is an error code
and you use a separate API to access the tracker for the most recent call
to put/get.


>       }
>
>
>       status
>       {
>         CURRENT_TEXT
>         {
>           Gets the last known remote state of the delivery associated with
>           the given tracker.
>         }
>
>         NEW_TEXT
>         {
>           Find the current delivery status of the outgoing message
>           associated with this tracker, as long as the message is still
>           within your outgoing window.
>         }
>

This should work for both incoming and outgoing deliveries.


>       }
>
>
>       settle
>       {
>         CURRENT_TEXT
>         {
>           // none
>         }
>
>         NEW_TEXT
>         {
>           Frees a Messenger from tracking the status associated with a
> given
>           tracker. If you don't supply a tracker, all outgoing messages up
>           to the most recent will be settled.
>         }
>       }
>
>
>       send
>       {
>         CURRENT_TEXT
>         {
>           Blocks until the outgoing queue is empty or the operation times
>           out. The L{timeout} property controls how long a L{Messenger}
> will
>           block before timing out.
>         }
>
>         NEW_TEXT
>         {
>           This call will block until the indicated number of messages have
> been
>           sent, or until the operation times out.  If n is -1 this call
> will
>           block until all outgoing messages have been sent. If n is 0 then
> this
>           call will send whatever it can without blocking.
>         }
>       }
>
>
>       recv
>       {
>         CURRENT_TEXT
>         {
>           Receives up to I{n} messages into the incoming queue of the
>           L{Messenger}. If I{n} is not specified, L{Messenger} will
> receive as many
>           messages as it can buffer internally. This method will block
> until at least
>           one message is available or the operation times out.
>         }
>
>         NEW_TEXT
>         {
>           Receives up to N messages into the incoming queue.  If no value
>           for N is supplied, this call will receive as many messages as it
>           can buffer internally.  If the messenger is in blocking mode,
> this
>           call will block until at least one message is available in the
>           incoming queue.
>         }
>       }
>
>
>       work
>       {
>         CURRENT_TEXT
>         {
>           // none
>         }
>
>         ADD_TEXT
>         {
>           Sends or receives any outstanding messages queued for a
> messenger.
>           This will block for the indicated timeout.
>

Strictly speaking this can do other work as well, e.g. if you're in non
blocking mode, then you could call messenger.stop() and then call
messenger.work() until messenger.stopped() returns true. This is doing I/O
work, but it is not necessarily sending/receivng messages. While it may end
up sending or receiving messages, it could also just do the I/O work
necessary to shut down all the connections.

I'm not sure how much detail we need here, but I figured I'd mention it for
completeness.


>         }
>       }
>
>
>       interrupt
>       {
>         CURRENT_TEXT
>         {
>           // none
>         }
>
>         ADD_TEXT
>         {
>           The messenger interface is single-threaded.
>           This is the only messenger function intended to be called
>           from outside of the messenger thread.
>           Call this from a non-messenger thread to interrupt
>           a messenger that is blocking.
>

You could add that this will cause any in progress blocking call to return
a PN_INTERRUPT code in C or throw the L{Interrupt} exception in python. If
there is no currently blocking call, then the next blocking call will be
effected, even if it is within the same thread that interrupt was called
from.

        }
>       }
>
>
>       get
>       {
>         CURRENT_TEXT
>         {
>           Moves the message from the head of the incoming message queue
> into
>           the supplied message object. Any content in the supplied message
>           will be overwritten.
>         }
>
>         ADD_TEXT
>         {
>           A tracker for the incoming message is returned.
>
>           If the given pointer to a message structure is NULL,
>           the message popped from the head of the queue is discarded.
>

That sounds like it's describing the C API, not the Python API. For Python
it would be "if None is passed in for the message object, the message
popped from the head of the queue is discarded." or something like that.


>         }
>       }
>
>
>       accept
>       {
>         CURRENT_TEXT
>         {
>           Accepts messages retreived from the incoming message queue.
>         }
>
>         NEW_TEXT
>         {
>           Signal the sender that you have acted on the message
>           pointed to by the tracker.  If no tracker is supplied,
>           then all messages that have been returned by the get method
>           are accepted, back to the limit of your incoming window size.
>

I'm not sure what you mean by "back to the limit of your incoming window
size", but as phrased it could be interpreted as the window putting a limit
on what gets accepted, which is a bit misleading. The incoming window
controls how many incoming messages the messenger will track before auto
settling, i.e. what we need to get across is along the lines "If no tracker
is supplied, then all messages that have been returned by the get method
(and not auto-settled based on the incoming window) are accepted." Except
maybe less tortured phrasing. ;-)


>         }
>       }
>
>
>       reject
>       {
>         CURRENT_TEXT
>         {
>           Rejects messages retreived from the incoming message queue.
>         }
>
>         NEW_TEXT
>         {
>           Rejects the message indicated by the tracker.  If no tracker
>           is supplied, all messages that have been returned by the get
> method
>           re rejected, back to the limit of your incoming window size.
>

Same as above.


>         }
>       }
>
>
>       route
>       {
>         CURRENT_TEXT
>         {
>           // none
>         }
>
>         NEW_TEXT
>         {
>           Adds a routing rule to a Messenger's internal routing table.
>
>           The route procedure may be used to influence how a messenger will
>           internally treat a given address or class of addresses. Every
> call
>           to the route procedure will result in messenger appending a
> routing
>           rule to its internal routing table.
>
>           Whenever a message is presented to a messenger for delivery, it
>           will match the address of this message against the set of routing
>           rules in order. The first rule to match will be triggered, and
>           instead of routing based on the address presented in the message,
>           the messenger will route based on the address supplied in the
> rule.
>
>           The pattern matching syntax supports two types of matches, a '%'
>           will match any character except a '/', and a '*' will match any
>           character including a '/'.
>
>           A routing address is specified as a normal AMQP address, however
> it
>           may additionally use substitution variables from the pattern
> match
>           that triggered the rule.
>
>           Any message sent to "foo" will be routed to "amqp://foo.com":
>
>             self.client.route("foo", "amqp://foo.com");
>
>           Any message sent to "foobar" will be routed to
>           "amqp://foo.com/bar":
>
>             self.client.route("foobar", "amqp://foo.com/bar");
>
>           Any message sent to bar/<path> will be routed to the
> corresponding
>           path within the amqp://bar.com domain:
>
>             self.client.route("bar/*", "amqp://bar.com/$1");
>
>           Route all messages over TLS:
>
>             self.client.route("amqp:*", "amqps:$1")
>
>           Supply credentials for foo.com:
>
>             self.client.route("amqp://foo.com/*", "amqp://
> user:passw...@foo.com/$1");
>
>           Supply credentials for all domains:
>
>             self.client.route("amqp://*", "amqp://user:password@$1");
>
>           Route all addresses through a single proxy while preserving the
>           original destination:
>
>             self.client.route("amqp://%/*", "amqp://user:password@proxy
> /$1/$2");
>
>           Route any address through a single broker:
>
>             self.client.route("*", "amqp://user:password@broker/$1");
>
>         }
>
>         NOTE
>         {
>           This is taken directly from original text in the C API -- I only
>           changed pn_messenger_route  to  selfie.client.route everywhere.
>

I would use messenger.route(), not self.client.route().


>         }
>       }
>
>
>       rewrite
>       {
>         CURRENT_TEXT
>         {
>           // none
>         }
>
>         NEW_TEXT
>         {
>           Similar to route(), except that the destination of
>           the message is determined before the message address is
> rewritten.
>
>           The outgoing address is only rewritten after routing has been
>           finalized.  If a message has an outgoing address of
>           "amqp://0.0.0.0:5678", and a rewriting rule that changes its
>           outgoing address to "foo", it will still arrive at the peer that
>           is listening on "amqp://0.0.0.0:5678", but when it arrives
> there,
>           the receiver will see its outgoing address as "foo".
>
>           The default rewrite rule removes username and password from
> addresses
>           before they are transmitted.
>         }
>       }
>
>     }
>
>
>
>
>     Property Details
>     {
>       name
>       {
>         CURRENT_TEXT
>         {
>           The name of the L{Messenger}.
>         }
>
>         NEW_TEXT
>         {
>           // no change
>         }
>       }
>
>
>       certificate
>       {
>         CURRENT_TEXT
>         {
>           Path to a certificate file for the L{Messenger}. This
> certificate is
>           used when the L{Messenger} accepts or establishes SSL/TLS
> connections.
>           This property must be specified for the L{Messenger} to accept
>           incoming SSL/TLS connections and to establish client
> authenticated
>           outgoing SSL/TLS connection. Non client authenticated outgoing
> SSL/TLS
>           connections do not require this property.
>         }
>
>         NEW_TEXT
>         {
>           // no change
>         }
>       }
>
>
>       private_key
>       {
>         CURRENT_TEXT
>         {
>           Path to a private key file for the L{Messenger's<Messenger>}
>           certificate. This property must be specified for the
> L{Messenger} to
>           accept incoming SSL/TLS connections and to establish client
>           authenticated outgoing SSL/TLS connection. Non client
> authenticated
>           SSL/TLS connections do not require this property.
>         }
>
>         NEW_TEXT
>         {
>           // no change
>         }
>       }
>
>
>       password
>       {
>         CURRENT_TEXT
>         {
>           This property contains the password for the
> L{Messenger.private_key}
>           file, or None if the file is not encrypted.
>         }
>
>         NEW_TEXT
>         {
>           // no change
>         }
>       }
>
>
>       trusted_certificates
>       {
>         CURRENT_TEXT
>         {
>           A path do a database of trusted certificates for use in
> verifying the
>           peer on an SSL/TLS connection. If this property is None, then
> the peer
>           will not be verified
>         }
>
>         NEW_TEXT
>         {
>           // no change
>         }
>       }
>
>
>       timeout
>       {
>         CURRENT_TEXT
>         {
>           The timeout property contains the default timeout for blocking
>           operations performed by the L{Messenger}.
>         }
>       }
>
>
>       blocking
>       {
>         CURRENT_TEXT
>         {
>           // none
>         }
>
>         NEW
>         {
>           Enable or disable blocking behavior during message sending
>           and receiving.
>

As above, this impacts every blocking API call with the exception of
work(). (Currently I believe that would just be send, recv, and stop,
however that list may grow in the future.)


>         }
>       }
>
>
>       incoming_window
>       {
>         CURRENT_TEXT
>         {
>           The incoming tracking window for the messenger. The messenger
> will
>           track the remote status of this many incoming deliveries after
> they
>           have been accepted or rejected. Defaults to zero.
>         }
>
>         ADD
>         {
>           Messages enter this window only when you
>           take them into your application using get().
>           If your incoming window size is N, and you get N+1 messages
> without
>           explicitly accepting or rejecting the oldest message, then it
> will be
>           implicitly accepted when it falls off the edge of the incoming
> window.
>

Strictly speaking it will be settled, with a null disposition. That will
only mean the same thing as being accepted if the default disposition for
the link is ACCEPTED. This will often be the case, but not always, e.g. a
broker could decide that the default disposition is to release the message
back onto the queue.

        }
>       }
>
>
>       outgoing_window
>       {
>         CURRENT_TEXT
>         {
>           The outgoing tracking window for the messenger. The messenger
> will
>           track the remote status of this many outgoing deliveries after
> calling
>           send. Defaults to zero.
>         }
>
>         ADD
>         {
>           A message enters this window when you call the put() method with
> the
>           message.  If your outgoing window size is N, and you call put()
> N+1
>           times, new status information will no longer be available for the
>           first message.
>

That should probably be "no" status information, not "new" status
information.


>         }
>       }
>
>
>       stopped
>       {
>         CURRENT_TEXT
>         {
>           // none
>         }
>
>         NEW
>         {
>           Returns true iff a messenger is in the stopped state.
>           This function does not block.
>         }
>       }
>
>
>       outgoing
>       {
>         CURRENT_TEXT
>         {
>           The outgoing queue depth.
>         }
>
>         NEW_TEXT
>         {
>           // no change
>         }
>       }
>
>
>       incoming
>       {
>         CURRENT_TEXT
>         {
>           The incoming queue depth.
>         }
>
>         NEW_TEXT
>         {
>           // no change
>         }
>       }
>
>     }
>
>
>
>
>
>

Reply via email to