feat(config): support endpoints grouped by dirs

coco/client.py

@@ -23,10 +23,10 @@
         `CliGroup.init_coco` to fetch the coco config from the daemon, using the
         "--backend" and/or "--conf" options previously found (or envars) to
         determine the host/port of the daemon.  Once the config is returned,
-        this function creates `EndpointCommand` instances for each endpoint
-        defined, and adds them all to the `CliGroup` instance.  If an error
-        occurs trying to contact the daemon, the error is recorded for later,
-        but program flow continues.
+        this function creates `EndpointCommand` or `EndpointGroup` instances
+        for each endpoint/endpoint tree deefined, and adds them all to the
+        `CliGroup` instance.  If an error occurs trying to contact the daemon,
+        the error is recorded for later, but program flow continues.
     7. After creating all the endpoint commands, control passes to the normal
         `click.Group.list_commands` or `click.Group.get_command` and program
         flow now follows in the normal `click` way.
@@ -51,6 +51,42 @@
 from .endpoint import VALUE_TYPE
 
 
+class EndpointGroup(click.Group):
+    """A click Group for an endpoint tree.
+
+    NB: This is not used for the top-level of the endpoint tree.  See the
+    CliGroup instead.
+    """
+
+    def __init__(self, tree):
+        self.tree = tree
+        name = tree["name"]
+
+        # Figure out short help.
+        if "summary" in tree:
+            # If a there's a "summary" use that.
+            short_help = tree["summary"]
+        elif "description" in tree:
+            # If there's a description, use everything up to the first full stop.
+            short_help = tree["description"]
+            try:
+                short_help = short_help[: short_help.index(". ")]
+            except ValueError:
+                # description is at most once sentence.  Use it all.
+                pass
+
+        help_ = tree.get("description", tree.get("summary", "NO DESCRIPTION"))
+
+        # Generate all the subcommands / subgroups
+        commands = [
+            EndpointGroup(endpoint) if endpoint["tree"] else EndpointCommand(endpoint)
+            for endpoint in tree["endpoints"]
+        ]
+
+        # click initialisation
+        super().__init__(name, short_help=short_help, help=help_, commands=commands)
+
+
 class EndpointCommand(click.Command):
     """A click Command for a single endpoint."""
 
@@ -76,11 +112,10 @@ def __init__(self, endpoint):
 
         params, param_help = self._endpoint_params()
 
-        help_ = (
-            endpoint.get("description", endpoint.get("summary", "NO DESCRIPTION"))
-            + "\n\n\b\n"
-            + param_help
-        )
+        help_ = endpoint.get("description", endpoint.get("summary", "NO DESCRIPTION"))
+
+        if param_help:
+            help_ += "\n\n\b\n" + param_help
 
         # click initialisation
         super().__init__(
@@ -314,7 +349,11 @@ def __init__(self, *args, **kwargs):
     def _init_endpoints(self, endpoints):
         """Update the click command list with endpoint definitions."""
         for endpoint in endpoints:
-            self.add_command(EndpointCommand(endpoint))
+            self.add_command(
+                EndpointGroup(endpoint)
+                if endpoint["tree"]
+                else EndpointCommand(endpoint)
+            )
 
     def format_commands(self, ctx, formatter):
         """List commands and endpoints.
@@ -330,7 +369,7 @@ def format_commands(self, ctx, formatter):
             if cmd is None or cmd.hidden:
                 continue
 
-            if isinstance(cmd, EndpointCommand):
+            if isinstance(cmd, (EndpointCommand, EndpointGroup)):
                 endpoints.append((subcommand, cmd))
             else:
                 commands.append((subcommand, cmd))

coco/config.py

@@ -83,11 +83,9 @@
     # These specify the logger path, the minimum level it applies to and the
     # slack channel the messages should go to.
     slack_rules:
-
         - logger: coco
           level: WARNING
           channel: coco-alerts
-
         - logger: coco.endpoint.update-pulsar-pointing-0
           level: INFO
           channel: pulsar-timing-ops
@@ -223,11 +221,16 @@ def load_config(
     if not any_exist:
         raise click.ClickException("No configuration files available.")
 
+    # Validate config
     _validate_and_resolve(config)
 
     # Local endpoints are not loaded in the CLI
     if not cli:
-        _load_endpoint_config(config)
+        # Load the endpoints
+        endpoint_tree = load_endpoint_tree(Path(config["endpoint_dir"]))
+
+        # We only record the endpoint list per se, not the whole top-level group
+        config["endpoints"] = endpoint_tree["endpoints"]
 
     return config
 
@@ -301,38 +304,83 @@ def _validate_and_resolve(config: dict) -> None:
         )
 
 
-def _load_endpoint_config(config: dict) -> None:
-    """Load the endpoint config.
+def load_endpoint(path: Path) -> dict | None:
+    """Read an endpoint from `path`.
 
-    The config is injected into the passed in config object.
+    Returns the parsed endpoint config entry, or None if `path` wasn't an
+    endpoint file
     """
-    config["endpoints"] = []
+    # A file called "__meta.conf" (two underscores) can be used to set metadata
+    # about the containing endpoint group.
+    meta = path.name == "__meta.conf"
 
-    endpoint_dir = Path(config["endpoint_dir"])
+    # Only accept files ending in .conf as endpoint configs.
+    # Endpoint config files starting with an underscore (_) are disabled (except
+    # a __meta.conf file).
+    if not meta and (path.suffix != ".conf" or path.name.startswith("_")):
+        # Not a valid endpoint file
+        logger.debug(f"Ignoring invalid/disabled endpoint {path}.")
+        return None
 
-    for endpoint_file in endpoint_dir.iterdir():
-        # Only accept files ending in .conf as endpoint configs.
-        # Endpoint config files starting with an underscore (_) are disabled.
-        if endpoint_file.suffix == ".conf" and not endpoint_file.name.startswith("_"):
-            logger.debug(f"Loading endpoint config {endpoint_file}.")
+    logger.debug(f"Loading endpoint config {path}.")
 
-            # Remove .conf from the config file name to get the name of the endpoint
-            name = endpoint_file.stem
+    try:
+        with path.open("r", encoding="utf-8") as fh:
+            conf = yaml_load(fh)
+    except (OSError, ValueError, UnicodeDecodeError, yaml.YAMLError) as e:
+        raise click.ClickException(f"Failure reading endpoint {path}: {e}") from e
 
-            try:
-                with endpoint_file.open("r", encoding="utf-8") as fh:
-                    conf = yaml_load(fh)
-            except (OSError, ValueError, UnicodeDecodeError, yaml.YAMLError) as e:
-                raise click.ClickException(
-                    f"Failure reading endpoint {endpoint_file}: {e}"
-                ) from e
+    # Extra endpoint metadata
+    conf["meta"] = meta
+    conf["tree"] = False
 
-            # A "name" field in an endpoint file is not allowed
-            if "name" in conf:
-                raise click.ClickException(
-                    f"spurious 'name' found in endpoint {name!r}"
-                )
-            conf["name"] = name
+    # A "name" field in an endpoint file is not allowed
+    if "name" in conf:
+        raise click.ClickException(f"spurious 'name' found in endpoint {path}")
 
-            # Endpoint config will be validated after the state is loaded
-            config["endpoints"].append(conf)
+    # Remove .conf from the config file name to get the name of the endpoint
+    conf["name"] = path.stem
+
+    return conf
+
+
+def load_endpoint_tree(path):
+    """Iterate over a directory `path` in the endpoint tree.
+
+    Returns an endpoint tree config for the directory,
+    containing the endpoints and subtrees found within.
+    """
+    tree = {
+        "name": path.name,
+        "tree": True,
+        "description": "Unremarkable endpoint tree",
+        "endpoints": [],
+    }
+    for dir_entry in path.iterdir():
+        if dir_entry.is_dir():
+            # Parse the subtree
+            subtree = load_endpoint_tree(dir_entry)
+
+            # The subtree is only added if it's not empty
+            if subtree["endpoints"]:
+                tree["endpoints"].append(subtree)
+        else:
+            endpoint = load_endpoint(dir_entry)
+
+            # Skip if nothing was loaded
+            if not endpoint:
+                continue
+
+            if endpoint["meta"]:
+                # If this was a __meta.conf file, merge (select) data into the
+                # tree config
+                for key in {"description", "summary"}:
+                    if key in endpoint:
+                        tree[key] = endpoint[key]
+            else:
+                # otherwise, delete the "meta" key and append endpoint
+                # to the tree's list
+                del endpoint["meta"]
+                tree["endpoints"].append(endpoint)
+
+    return tree

coco/core.py

@@ -107,11 +107,44 @@ def __init__(
         # Now that the state is loaded, validate all the endpoints defined by the config
         endpoints = []
         groups = self.config["groups"]
-        all_endpoints = {endpoint["name"] for endpoint in self.config["endpoints"]}
+
+        # dict of all endpoint upaths
+        upaths = {}
+
+        def _list_all_endpoints(parent: str, endpoints: list) -> set:
+            """Generate a set of all endpoint names.
+
+            Also ensures no two endpoints end up with the
+            same upath.
+            """
+            all_ = set()
+
+            if parent:
+                parent += "/"
+
+            for endpoint in endpoints:
+                path = parent + endpoint["name"]
+                if endpoint["tree"]:  # Descend through subtree
+                    all_ |= _list_all_endpoints(path, endpoint["endpoints"])
+                else:
+                    # Having two endpoints called, say, "alpha_beta/gamma" and
+                    # "alpha/beta_gamma" is not allowed, to keep the redis keys
+                    # sensible.
+                    upath = path.replace("/", "_")
+                    if upath in upaths:
+                        raise click.ClickException(
+                            f"Endpoint clash: endpoints {path!r} and "
+                            f"{upaths[upath]!r} can't both be defined."
+                        )
+                    upaths[upath] = path
+                    all_.add(path)
+            return all_
+
+        all_endpoints = _list_all_endpoints("", self.config["endpoints"])
 
         for endpoint in self.config["endpoints"]:
             endpoints.append(
-                validate_endpoint(endpoint, groups, all_endpoints, self.state)
+                validate_endpoint(endpoint, "", groups, all_endpoints, self.state)
             )
         self.config["endpoints"] = endpoints
 
@@ -133,7 +166,8 @@ def __init__(
 
         self._config_slack_loggers()
 
-        self._load_endpoints()
+        self.endpoints = {}
+        self._load_endpoints("", self.config["endpoints"])
         self._local_endpoints()
         self._check_endpoint_links()
 
@@ -428,18 +462,26 @@ def _load_config(self, config_path: os.PathLike | None, testing: bool):
                         f"Required key {key} missing from slack rule: {rdict}."
                     )
 
-    def _load_endpoints(self):
-        self.endpoints = {}
+    def _load_endpoints(self, parent, endpoints):
+        if parent:
+            parent += "/"
 
-        for conf in self.config["endpoints"]:
+        for conf in endpoints:
             name = conf["name"]
 
-            # Create the endpoint object
-            self.endpoints[name] = Endpoint(name, conf, self.forwarder, self.state)
+            # Descend through endpoint subtrees
+            if conf["tree"]:
+                self._load_endpoints(parent + name, conf["endpoints"])
+                continue
+
+            # Otherwise, create the endpoint object
+            self.endpoints[name] = Endpoint(
+                name, parent, conf, self.forwarder, self.state
+            )
 
             if not self.endpoints[name].has_external_forwards:
                 logger.debug(
-                    f"Endpoint {name} has `call` set to 'null'. This means it "
+                    f"Endpoint {parent}{name} has `call` set to 'null'. This means it "
                     f"doesn't call external endpoints. It might check other coco "
                     f"endpoints or return some part of coco's state."
                 )