Merge pull request #220 from simonsobs/core-fixes

Some bug fixes and a bunch of OpSession documentation work

docs/conf.py

@@ -45,6 +45,11 @@
 ]
 extensions += ['sphinxarg.ext']
 
+# Present auto-documented members in source order (rather than alphabetical).
+autodoc_default_options = {
+    'member-order': 'bysource',
+}
+
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ['_templates']
 

docs/developer/clients.rst

@@ -190,11 +190,12 @@ Client example would be::
 Replies from Operation methods
 ------------------------------
 
-The responses from Operation methods is a tuple, (status, message,
-session).  The elements of the tuple are:
+The response from Operation methods is a tuple, ``(status, message,
+session)``.  The elements of the tuple are:
 
   ``status``
-    An integer value equal to ocs.OK, ocs.ERROR, or ocs.TIMEOUT.
+    An integer value equal to ocs.OK, ocs.ERROR, or ocs.TIMEOUT (see
+    :class:`ocs.base.ResponseCode`).
 
   ``message``
     A string providing a brief description of the result (this is
@@ -205,8 +206,8 @@ session).  The elements of the tuple are:
     The session information... see below.
 
 Responses obtained from MatchedClient calls are lightly wrapped by
-class ``OCSReply`` so that ``__repr__`` produces a nicely formatted
-description of the result.  For example::
+class :class:`ocs.matched_client.OCSReply` so that ``__repr__``
+produces a nicely formatted description of the result.  For example::
 
   >>> c.set_autoscan.wait()
   OCSReply: OK : Operation "set_autoscan" just exited.
@@ -220,43 +221,16 @@ description of the result.  For example::
 
 
 The ``session`` portion of the reply is dictionary containing a bunch
-of potentially useful information.  This information corresponds to
-the OpSession maintained by the OCSAgent class for each run of an
-Agent's Operation (see OpSession in ocs/ocs_agent.py):
-
-  ``'session_id'``
-    An integer identifying this run of the Operation.  If an Op ends
-    and is started again, ``session_id`` will be different.
-
-  ``'op_name'``
-    The operation name.  You probably already know this.
-
-  ``'status'``
-    A string representing the state of the operation.  The possible
-    values are 'starting', 'running', 'done'.
-
-  ``'start_time'``
-    The timestamp corresponding to when this run was started.
-
-  ``'end_time'``
-    If ``status`` == ``'done'``, then this is the timestamp at which
-    the run completed.  Otherwise it will be None.
-
-  ``'success'``
-    If ``status`` == ``'done'``, then this is a boolean indicating
-    whether the operation reported that it completed successfully
-    (rather than with an error).
-
-  ``'data'``
-    Agent-specific data that might of interest to a user.  This may be
-    updated while an Operation is running, but once ``status`` becomes
-    ``'done'`` then ``data`` should not change any more.  A typical
-    use case here would be for a Process that is monitoring some
-    system to report the current values of key parametrs.  This should
-    not be used as an alternative to providing a data feed... rather
-    it should provide current values to answer immediate questions.
-
-  ``'messages'``
-    A list of Operation log messages.  Each entry in the list is a
-    tuple, (timestamp, text).
+of potentially useful information, such as timestamps for the
+Operation run's start and end, a success code, and a custom data
+structure populated by the Agent.
 
+The information can be accessed through the OCSReply, for example::
+
+  >>> reply = c.set_autoscan.status()
+  >>> reply.session['start_time']
+  1585667844.423
+
+For more information on the contents of ``.session``, see the
+docstring for :func:`ocs.ocs_agent.OpSession.encoded`, and the Data
+Access section on :ref:`session_data`.

docs/developer/session-data.rst

@@ -1,11 +1,14 @@
+.. _session_data:
+
 session.data
 ============
 
 Data Feeds make use of the crossbar Pub/Sub functionality to pass data around
 the network, however, sometimes you might not want to receive all data, just
-the most recent values. For this purpose there is the OpSession data attribute.
-This is per OCS operation location to store recent data of interest to OCS
-Agent users.
+the most recent values. For this purpose there is the ``session.data`` attribute.
+This is a per OCS operation location to store recent data of interest to OCS
+Agent users.  The ``session`` argument passed to each Operation function is an
+object of class :class:`ocs.ocs_agent.OpSession`.
 
 Often this is used to store the most recent values that are queried by the
 Agent, for example the temperature of thermometers on a Lakeshore device. This
@@ -141,10 +144,25 @@ is lines 95-100::
 
 
 This block formats the latest values for each "channel" into a dictionary and
-stores it in ``session.data``. The format of the data stored in
-``session.data`` is left to the Agent developer, and should be documented in
-the docstring for the Process. This documentation should include a description
-of what you are including and what that information means.
+stores it in ``session.data``.
+
+
+The structure of the ``data`` entry is not strictly defined, but
+please observe the following guidelines:
+
+- Document your ``data`` structure in the Operation docstring.
+- Provide a `timestamp` with the readings, or with each group of
+  readings, so that the consumer can confirm they're recent.
+- The session data is passed to clients with every API response, so
+  avoid storing a lot of data in there (as a rule of thumb, try to
+  keep it < 100 kB).
+- Fight the urge to store timestreams (i.e. a history of recent
+  readings) -- try to use data feeds for that.
+- When data are so useful that they are used by other clients /
+  control scripts to make decisions in automated contexts, then they
+  should also be pushed out to a data feed, so that there is a full
+  record of all variables that were affecting system behavior.
+
 
 .. note::
     You should consider the desired structure carefully, as future changes the

ocs/__init__.py

@@ -1,4 +1,4 @@
-from .base import RET_VALS, OK, ERROR, TIMEOUT
+from .base import OK, ERROR, TIMEOUT, ResponseCode, OpCode
 
 from . import site_config
 

ocs/base.py

@@ -1,24 +1,77 @@
 import ocs
 from enum import Enum
 
-RET_VALS = {
-    'OK': 0,
-    'ERROR': -1,
-    'TIMEOUT': 1,
-}
+class ResponseCode(Enum):
+    """Enumeration of response codes from the Operation API (start, stop,
+    wait, ...).
 
-OK = RET_VALS['OK']
-ERROR = RET_VALS['ERROR']
-TIMEOUT = RET_VALS['TIMEOUT']
+    These response codes indicate only whether the API call was
+    successful, in that the request was propagated all the way to the
+    Agent's Operation code.  They are not used to represent success or
+    failure of the Operation itself.
 
-class OpCode(Enum):
     """
-    Enumeration of OpSession states.
+
+    #: OK indicates the request was successful.
+    OK = 0
+
+    #: ERROR indicates that the request could not be propagated fully.
+    #: This may occur, for example, if an invalid Operation name is
+    #: passed, if a request is made that conflicts with an Operation's
+    #: current state (e.g. .start() is called on an already-running
+    #: Operation), if an API call is made to a Operation of an
+    #: incompatible type (e.g. .stop() on a Task), or due to API
+    #: syntax error (e.g. misspelled keyword argument to .wait()).
+    ERROR = -1
+
+    #: TIMEOUT is returned in the case that a Client issued a blocking
+    #: call with timeout (.wait()), and the timeout expired before the
+    #: Operation completed.
+    TIMEOUT = 1
+
+
+OK = ResponseCode.OK.value
+ERROR = ResponseCode.ERROR.value
+TIMEOUT = ResponseCode.TIMEOUT.value
+
+class OpCode(Enum):
+    """Enumeration of OpSession "op_code" values.
+
+    The op_code corresponds to the session.status, except that if the
+    session.status == "done" then the op_code will be assigned a value
+    of either SUCCEEDED or FAILED based on session.success.
+
     """
+
+    #: NONE is used to represent an uninitialized OpSession, and does
+    #: not correspond to some attempt to run the Operation.
     NONE = 1
+
+    #: STARTING indicates that start() has been successfully called,
+    #: but the Operation has not yet marked itself as successfully
+    #: launched.  If this state is reached, then at the very least the
+    #: start request was not rejected because it was already running.
     STARTING = 2
+
+    #: RUNNING indicates that the Operation has performed its basic
+    #: initialization and parameter checking and is performing its
+    #: task.  Operation codes need to explicitly mark themselves as
+    #: running by calling session.set_state('running').
     RUNNING = 3
+
+    #: STOPPING indicates that the Agent has received a stop or abort
+    #: request for this Operation and will try to wrap things up ASAP.
     STOPPING = 4
+
+    #: SUCCEEDED indicates that the Operation has terminated and has
+    #: indicated the Operation was successful.  This includes the case
+    #: of a Process that was asked to stop and has shut down cleanly.
     SUCCEEDED = 5
+
+    #: FAILED indicates that the Operation has terminated with some
+    #: kind of error.
     FAILED = 6
+
+    #: EXPIRED may used to mark session information as invalid in cases
+    #: where the state cannot be determined.
     EXPIRED = 7

ocs/matched_client.py

@@ -15,8 +15,8 @@ class MatchedOp:
         def start(self, **kwargs):
             return OCSReply(*client.request('start', name, params=kwargs))
 
-        def wait(self):
-            return OCSReply(*client.request('wait', name))
+        def wait(self, timeout=None):
+            return OCSReply(*client.request('wait', name, timeout=timeout))
 
         def status(self):
             return OCSReply(*client.request('status', name))
@@ -122,8 +122,10 @@ def humanized_time(t):
 class OCSReply(collections.namedtuple('_OCSReply',
                                       ['status', 'msg', 'session'])):
     def __repr__(self):
-        ok_str = {ocs.OK: 'OK', ocs.ERROR: 'ERROR',
-                  ocs.TIMEOUT: 'TIMEOUT'}.get(self.status, '???')
+        try:
+            ok_str = ocs.ResponseCode(self.status).name
+        except ValueError:
+            ok_str = '???'
         text = 'OCSReply: %s : %s\n' % (ok_str, self.msg)
         if self.session is None or len(self.session.keys()) == 0:
             return text + '  (no session -- op has never run)'
@@ -143,7 +145,7 @@ def __repr__(self):
                 if s['success']:
                     run_str += ' without error'
                 else:
-                    run_str += ' with error'
+                    run_str += ' with ERROR'
                 run_str += ' %s ago, took %s' % (
                     humanized_time(time.time() - s['end_time']),
                     humanized_time(s['end_time'] - s['start_time']))