Remove old agent plugin system (#407)

BrianJKoopman · web-flow · commit 63fd5ef70735 · 2024-09-20T13:46:34.000-05:00
* Remove old plugin system and any usage of it

* Remove old plugin system docs and use of 'agent-paths' config

* Remove use of 'agent_script' when launching plugin

* Update 'agent_script' docstring

* Add autodoc for drivers to HostManager agent docs

* Remove use of 'launcher script' in logs
diff --git a/docs/agents/host_manager.rst b/docs/agents/host_manager.rst
@@ -59,3 +59,7 @@ Supporting APIs
 ---------------
 
 .. automethod:: ocs.agents.host_manager.agent.HostManager._reload_config
+
+.. automodule:: ocs.agents.host_manager.drivers
+    :members:
+    :show-inheritance:
diff --git a/docs/api.rst b/docs/api.rst
@@ -54,6 +54,7 @@ agents.host_manager
     :members:
     :undoc-members:
     :show-inheritance:
+    :noindex:
 
 agents.influxdb_publisher
 `````````````````````````
diff --git a/docs/developer/site_config.rst b/docs/developer/site_config.rst
@@ -90,9 +90,6 @@ the CrossbarConfig class:
     :members: from_dict
     :noindex:
 
-The significance of ``agent-paths`` is described more in
-:ref:`agent_plugins`.
-
 InstanceConfig
 --------------
 
@@ -205,43 +202,3 @@ river_ctrl.py in the examples):
       print('Starting a data acquisition process...')
       d1 = yield cw.start()
       #...
-
-
-.. _agent_plugins:
-
-Agent Script Discovery
-======================
-
-Agent scripts are currently written as stand-alone python scripts (and
-thus not not importable through the main ``ocs`` python module).  To
-support automatic launching of Agents, ``site_config`` includes a
-plugin-style system to register Agent scripts.  This is flexible
-enough to support both the natively packaged Agents and any
-"third-party" agents needed in a particular installation.  The system
-works like this:
-
-- A bundle of Agent scripts is assembled at some location.  There are
-  no restrictions on where these scripts can live in the file system.
-  For example, a script called ``riverbank_agent.py`` might live in
-  ``/simonsobs/agents/``.
-- A special "registration script" is written, with a filename of the
-  form ``ocs_plugin_*.py``.  This script should live in Python's
-  import path, or else (and this is better), the path to the script
-  should be included in the ``agent-paths`` variable in the SCF for
-  this host.  For example, we might put the file in ``/simonsobs/agents``
-  and call it ``ocs_plugin_simonsobs.py``.
-- When the site_config system (specifically the HostManager agent)
-  needs to find a particular agent script, it:
-
-  - Adds any directories in ``agent-paths`` to the Python import path.
-  - Scans through all importable modules, and imports them if they
-    match the ``ocs_plugin_*`` name pattern.
-
-- The ``ocs_plugin`` script makes calls into ocs to associate a
-  particular script filenames to agent class names.  In our example,
-  ``ocs_plugin_simonsobs.py`` would call
-  ``ocs.site_config.register_agent_class('RiverBank',
-  '/simonsobs/agents/riverbank_agent.py')``.
-
-A good example of a plugin script can be found in the OCS agents
-directory, ``ocs_plugins_standard.py``.
diff --git a/docs/user/centralized_management.rst b/docs/user/centralized_management.rst
@@ -78,11 +78,6 @@ block would become:
       # Directory for logs.
       'log-dir': '/simonsobs/log/ocs/',
 
-      # List of additional paths to Agent plugin modules.
-      'agent-paths': [
-        '/simonsobs/ocs/agents/',
-      ],
-
       # Description of host-1's Agents.
       # We have two readout devices; they are both Lakeshore 240. But they can
       # be distinguished, on startup, by a device serial number.
diff --git a/docs/user/quickstart.rst b/docs/user/quickstart.rst
@@ -55,12 +55,6 @@ structure.
         # Directory for logs.
         'log-dir': '/home/<user>/log/ocs/',
 
-        # List of additional paths to Agent plugin modules.
-        'agent-paths': [
-            '/home/<user>/git/ocs/agents/',
-            '/home/<user>/git/socs/agents/',
-        ],
-
         # Agents running directly on the host machine
         # Note: We aren't going to run this Agent in the quickstart example,
         #       but this gives a good example of configuring an agent directly
diff --git a/docs/user/site_config.rst b/docs/user/site_config.rst
@@ -43,11 +43,6 @@ instances (running two different classes of agent):
       # Directory for logs.
       'log-dir': '/simonsobs/log/ocs/',
 
-      # List of additional paths to Agent plugin modules.
-      'agent-paths': [
-        '/simonsobs/ocs/agents/',
-      ],
-
       # Description of host-1's Agents.
       # We have two readout devices; they are both Lakeshore 240. But they can
       # be distinguished, on startup, by a device serial number.
@@ -89,11 +84,6 @@ instances (running two different classes of agent):
       # Directory for logs.
       'log-dir': '/simonsobs/log/ocs/',
 
-      # List of additional paths to Agent plugin modules.
-      'agent-paths': [
-        '/simonsobs/ocs/agents/',
-      ],
-
       # Description of host-2's Agents.
       # We have two devices; another Lakeshore 240, and the OCS g3 file
       # Aggregator.
diff --git a/example/miniobs/README.rst b/example/miniobs/README.rst
@@ -12,7 +12,7 @@ In particular:
 
 - The list of Agents is defined under hostname ``localhost``.  This is
   a wildcard that can only work in a single-host system.
-- ``log-dir``, ``agent-paths``, and the ``crossbar:config-dir`` are
+- ``log-dir`` and the ``crossbar:config-dir`` are
   all specified as relative paths.  Behavior will be unpredictable if
   Agents are not all invoked in the same working directory.
 
diff --git a/example/miniobs/default.yaml b/example/miniobs/default.yaml
@@ -11,10 +11,6 @@ hosts:
   localhost: {
     'crossbar': {'config-dir': './dot_crossbar/'},
     'log-dir': './logs',
-    'agent-paths': [
-        '../../agents/',
-        './',
-        ],
     'agent-instances': [
       {'agent-class': 'HostManager',
        'instance-id': 'hm1',
diff --git a/ocs/agents/host_manager/agent.py b/ocs/agents/host_manager/agent.py
@@ -68,17 +68,6 @@ def _get_local_instances(self):
                             f'likely syntax error: {e}')
             return returnValue((False, instances, warnings))
 
-        # Scan for agent scripts in (deprecated) script registry
-        try:
-            for p in hc.agent_paths:
-                if p not in sys.path:
-                    sys.path.append(p)
-            site_config.scan_for_agents()
-        except Exception as e:
-            warnings.append('Failed to scan for old plugin agents -- '
-                            f'likely plugin config problem: {e}')
-            return returnValue((False, instances, warnings))
-
         # Gather managed items from site config.
         for inst in hc.instances:
             if inst['instance-id'] in instances:
@@ -317,18 +306,13 @@ def same_base_class(a, b):
                 continue
             if instance['management'] == 'host':
                 cls = instance['agent_class']
-                # Check for the agent class in the plugin system;
-                # then check the (deprecated) agent script registry.
+                # Check for the agent class in the plugin system
                 if cls in agent_plugins:
                     session.add_message(f'Found plugin for "{cls}"')
                     instance['agent_script'] = '__plugin__'
                     instance['operable'] = True
-                elif cls in site_config.agent_script_reg:
-                    session.add_message(f'Found launcher script for "{cls}"')
-                    instance['agent_script'] = site_config.agent_script_reg[cls]
-                    instance['operable'] = True
                 else:
-                    session.add_message(f'No plugin (nor launcher script) '
+                    session.add_message('No plugin '
                                         f'found for agent_class "{cls}"!')
             elif instance['management'] == 'docker':
                 instance['agent_script'] = self.docker_service_prefix + iid
@@ -361,16 +345,11 @@ def _launch_instance(self, instance):
         else:
             iid = instance['instance_id']
             pyth = sys.executable
-            script = instance['agent_script']
-            if script == '__plugin__':
-                cmd = [pyth, '-m', 'ocs.agent_cli']
-            else:
-                cmd = [pyth, script]
-            cmd.extend([
-                '--instance-id', iid,
-                '--site-file', self.site_config_file,
-                '--site-host', self.host_name,
-                '--working-dir', self.working_dir])
+            cmd = [pyth, '-m', 'ocs.agent_cli',
+                   '--instance-id', iid,
+                   '--site-file', self.site_config_file,
+                   '--site-host', self.host_name,
+                   '--working-dir', self.working_dir]
             prot = hm_utils.AgentProcessHelper(iid, cmd)
         prot.up()
         instance['prot'] = prot
diff --git a/ocs/agents/host_manager/drivers.py b/ocs/agents/host_manager/drivers.py
@@ -26,8 +26,8 @@ class ManagedInstance(dict):
     - 'operable' (bool): indicates whether the instance can be
       manipulated (whether calls to up/down should be expected to
       work).
-    - 'agent_script' (str): Path to the launcher script (if host
-      system managed).  If docker-managed, this is the service name.
+    - 'agent_script' (str): The docker service_name, if docker-managed.
+      Otherwise, the string ``__plugin__`` to indicate it is host managed.
     - 'prot': The twisted ProcessProtocol object (if host system
       managed), or the DockerContainerHelper (if a docker container).
     - 'target_state' (state): The state we're trying to achieve (up or
diff --git a/ocs/agents/ocs_plugin_standard.py b/ocs/agents/ocs_plugin_standard.py
diff --git a/ocs/ocsbow.py b/ocs/ocsbow.py
@@ -5,6 +5,7 @@
 
 import ocs
 from ocs import client_http, ocs_client
+from ocs.agent_cli import build_agent_list
 
 import argparse
 import difflib
@@ -465,14 +466,10 @@ def print_config(args, site_config):
             return False
         print('ocs plugin detection')
         print('--------------------')
-        print('Scanning.')
-        for p in host.agent_paths:
-            print('  ... adding to path: %s' % p)
-            sys.path.append(p)
-        ocs.site_config.scan_for_agents()
+        agent_list = build_agent_list()
         print('Found:')
-        for k, v in ocs.site_config.agent_script_reg.items():
-            print('  %-20s : %s' % (k, v))
+        for k, v in agent_list.items():
+            print('  %-25s : %s' % (k, v['module']))
         print()
 
     if args.cfg_request == 'crossbar':
diff --git a/ocs/site_config.py b/ocs/site_config.py
@@ -69,7 +69,6 @@ class HostConfig:
     def __init__(self, name=None):
         self.instances = []
         self.name = name
-        self.agent_paths = []
         self.log_dir = None
         self.working_dir = os.getcwd()
         self.crossbar_timeout = None
@@ -86,10 +85,6 @@ def from_dict(cls, data, parent=None, name=None):
         ``agent-instances`` (required)
             A list of AgentConfig descriptions.
 
-        ``agent-paths`` (optional)
-            A list of additional paths where OCS is permitted to
-            search for Agent plugin modules.
-
         ``crossbar`` (optional)
             Settings to assist with starting / stopping / monitoring a
             crossbar server running on this host.  There is a single
@@ -109,7 +104,6 @@ def from_dict(cls, data, parent=None, name=None):
         self.parent = parent
         self.data = data
         self.instances = data['agent-instances']
-        self.agent_paths = data.get('agent-paths', [])
         self.crossbar = CrossbarConfig.from_dict(data.get('crossbar'))
         self.log_dir = data.get('log-dir', None)
         self.crossbar_timeout = data.get('crossbar_timeout', 10)
@@ -682,47 +676,6 @@ def get_control_client(instance_id, site=None, args=None, start=True,
     return client
 
 
-# We'll also keep the Agent script registry here.
-agent_script_reg = {}
-
-
-def register_agent_class(class_name, filename):
-    """Register an Agent script in the site_config registry.
-
-    Args:
-        class_name (str): The Agent class name, e.g. "HostManager".
-        filename (str): The full path to the script that launches an
-            instance of this Agent class.
-
-    """
-    agent_script_reg[class_name] = filename
-
-
-def scan_for_agents(do_registration=True):
-    """Identify and import ocs Agent plugin scripts.  This will find all
-    modules in the current module search path (sys.path) that begin
-    with the name 'ocs_plugin\\_'.
-
-    Args:
-        do_registration (bool): If True, the modules are imported,
-            which likely causes them to call register_agent_class on
-            each agent they represent.
-
-    Returns:
-        The list of discovered module names.
-
-    """
-    import pkgutil
-    import importlib
-    items = []
-    for modinfo in pkgutil.iter_modules():
-        if modinfo.name.startswith('ocs_plugin_'):
-            items.append(modinfo.name)
-            if do_registration:
-                importlib.import_module(modinfo.name)
-    return items
-
-
 def parse_args(agent_class=None, parser=None, args=None):
     """
     Function to parse site-config and agent arguments. This function takes
diff --git a/tests/agents/test_ocs_plugin_standard.py b/tests/agents/test_ocs_plugin_standard.py
diff --git a/tests/default.yaml b/tests/default.yaml
@@ -10,9 +10,6 @@ hosts:
 
   localhost: {
     'log-dir': './log/',
-    'agent-paths': [
-      '../agents/',
-    ],
     'agent-instances': [
       {'agent-class': 'HostManager',
        'instance-id': 'host-manager-1',
