Check a valid service name has been provided (#171)

- Remove some redundant parameters.
- Construct the known_services help string separately (we'll use it
again).
- Exit early if no service has been specified, instead of opening a
frontend with no service and quietly twiddling our thumbs.

No attempt is made here to migrate to argparse.ArgumentParser instead of
the deprecated optparse.OptionParser, though the 'required' argument of
'ArgumentParser.add_argument' would obviate the need for separately
checking whether the '--service' option has been invoked.

* Tidy up the service name resolution logic

At present, a user can grow accustomed to lazily providing lower-case
service names when invoking 'workflows.service' from the command line.
This works fine until you encounter the following situation:
- Known services include "DLSISPyB" and "DLSISPyBPIA";
- user specifies '-s dlsispyb', expecting this to be interpreted as
'DLSISPyB', or else expecting an error;
- not finding a perfect match, the current logic checks whether any
known service names, rendered in lower case, start with 'dlsispyb',
finds two such matches;
- rather than selecting either of these two matches, the program quietly
 continues to open a frontend and doesn't start any service;
- user sees so warning or error, program twiddles its thumbs until
terminated.

New logic:
1. Check for perfect match;
2. Failing that, check for case-insensitive match;
3. Failing that, check for case-sensitive partial matches.  If
ambiguous, exit with error.
4. Failing that, repeat 3 but now case-insensitive.
5. If still no matches, exit with error.

* Neaten up the if nots

Author: benjaminhwilliams

src/workflows/contrib/start_service.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import sys
+
 from optparse import SUPPRESS_HELP, OptionParser
 
 import workflows
@@ -62,6 +64,7 @@ def run(
 
         # Enumerate all known services
         known_services = sorted(workflows.services.get_known_services())
+        known_services_help = "Known services: " + ", ".join(known_services)
 
         if version:
             version = f"{version} (workflows {workflows.version()})"
@@ -76,15 +79,11 @@ def run(
         parser.add_option(
             "-s",
             "--service",
-            dest="service",
             metavar="SVC",
-            default=None,
-            help="Name of the service to start. Known services: "
-            + ", ".join(known_services),
+            help=f"Name of the service to start. {known_services_help}",
         )
         parser.add_option(
             "--liveness",
-            dest="liveness",
             action="store_true",
             default=False,
             help=(
@@ -95,14 +94,12 @@ def run(
         )
         parser.add_option(
             "--liveness-port",
-            dest="liveness_port",
             default=8000,
             type="int",
             help="Expose liveness check endpoint on this port.",
         )
         parser.add_option(
             "--liveness-timeout",
-            dest="liveness_timeout",
             default=30,
             type="float",
             help="Timeout for the liveness check (in seconds).",
@@ -111,17 +108,15 @@ def run(
             parser.add_option(
                 "-m",
                 "--metrics",
-                dest="metrics",
                 action="store_true",
                 default=False,
                 help=(
-                    "Record metrics for this service and expose them on the port defined by"
-                    "the --metrics-port option."
+                    "Record metrics for this service and expose them on the port "
+                    "defined by the --metrics-port option."
                 ),
             )
             parser.add_option(
                 "--metrics-port",
-                dest="metrics_port",
                 default=8080,
                 type="int",
                 help="Expose metrics via a prometheus endpoint on this port.",
@@ -137,6 +132,10 @@ def run(
         # Call on_parsing hook
         (options, args) = self.on_parsing(options, args) or (options, args)
 
+        # Exit with error if no service has been specified.
+        if not options.service:
+            parser.error(f"Please specify a service name. {known_services_help}")
+
         # Create Transport factory
         transport_factory = workflows.transport.lookup(options.transport)
 
@@ -155,17 +154,34 @@ def on_transport_preparation_hook():
 
         transport_factory = on_transport_preparation_hook
 
-        # When service name is specified, check if service exists or can be derived
-        if options.service and options.service not in known_services:
-            matching = [s for s in known_services if s.startswith(options.service)]
-            if not matching:
-                matching = [
-                    s
-                    for s in known_services
-                    if s.lower().startswith(options.service.lower())
-                ]
-            if matching and len(matching) == 1:
-                options.service = matching[0]
+        # When service name is specified, check if service exists or can be derived.
+        if options.service not in known_services:
+            # First check whether the provided service name is a case-insensitive match.
+            service_lower = options.service.lower()
+            match = {s.lower(): s for s in known_services}.get(service_lower, None)
+            match = (
+                [match]
+                if match
+                # Next, check whether the provided service name is a partial
+                # case-sensitive match.
+                else [s for s in known_services if s.startswith(options.service)]
+                # Next check whether the provided service name is a partial
+                # case-insensitive match.
+                or [s for s in known_services if s.lower().startswith(service_lower)]
+            )
+
+            # Catch ambiguous partial matches and exit with an error.
+            if len(match) > 1:
+                sys.exit(
+                    f"Specified service name {options.service} is ambiguous, partially "
+                    f"matching each of these known services: " + ", ".join(match)
+                )
+            # Otherwise, set the derived service name, if there's a unique match.
+            elif match:
+                (options.service,) = match
+            # Otherwise, exit with an error.
+            else:
+                sys.exit(f"Please specify a valid service name. {known_services_help}")
 
         kwargs.update(
             {