Apply fixes from code review

- add ServoStatuses.from_error method for use in Status.from_error
- fix flaws in exception group processing of Status.from_error
- fix check remedy dynamic coroutine not called before passing into create_task
- Update servo_error_from_group to return default_error for consistency
- remove unneeded end param from HELLO log
- kubernetes model comment/docstring cleanup

servo/api.py

@@ -54,6 +54,16 @@ class ServoStatuses(enum.StrEnum):
     aborted = "aborted"
     cancelled = "cancelled"
 
+    def from_error(error: Exception) -> ServoStatuses:
+        if isinstance(error, servo.errors.AdjustmentRejectedError):
+            return ServoStatuses.rejected
+        elif isinstance(error, servo.errors.EventAbortedError):
+            return ServoStatuses.aborted
+        elif isinstance(error, servo.errors.EventCancelledError):
+            return ServoStatuses.cancelled
+        else:
+            return ServoStatuses.failed
+
 
 Statuses = Union[OptimizerStatuses, ServoStatuses]
 
@@ -127,38 +137,28 @@ def from_error(
         cls, error: servo.errors.BaseError | ExceptionGroup, **kwargs
     ) -> "Status":
         """Return a status object representation from the given error (first if multiple in group)."""
-        reason = getattr(error, "reason", ...)
         if isinstance(error, ExceptionGroup):
             servo.logger.warning(
                 f"from_error executed on exceptiongroup {error}. May produce undefined behavior"
             )
             status = ServoStatuses.failed
             try:
-                main_err = servo.errors.ServoError.servo_error_from_group(
+                error = servo.errors.ServoError.servo_error_from_group(
                     exception_group=error
                 )
-                if isinstance(main_err, list):
-                    main_err = main_err[0]
-                reason = main_err.reason
-            except:
+                if error._additional_errors:
+                    additional_messages = [str(e) for e in error._additional_errors]
+                    kwargs["additional_messages"] = (
+                        kwargs.get("additional_messages", []) + additional_messages
+                    )
+            except Exception:
                 servo.logger.exception(
                     "Failed to derive exceptiongroup reason for api response"
                 )
                 pass
-        if isinstance(error, servo.errors.AdjustmentRejectedError):
-            status = ServoStatuses.rejected
-        elif isinstance(error, servo.errors.EventAbortedError):
-            status = ServoStatuses.aborted
-        elif isinstance(error, servo.errors.EventCancelledError):
-            status = ServoStatuses.cancelled
-        else:
-            status = ServoStatuses.failed
 
-        if error._additional_errors:
-            additional_messages = [str(e) for e in error._additional_errors]
-            kwargs["additional_messages"] = (
-                kwargs.get("additional_messages", []) + additional_messages
-            )
+        reason = getattr(error, "reason", ...)
+        status = ServoStatuses.from_error(error)
 
         return cls(status=status, message=str(error), reason=reason, **kwargs)
 

servo/checks.py

@@ -783,20 +783,24 @@ async def process_checks(
 
                     if failure.remedy and checks_config.remedy:
                         try:
+                            coro = None
                             async with asyncio.timeout(10.0):
-                                if asyncio.iscoroutinefunction(failure.remedy):
-                                    awaitable = failure.remedy()
-                                elif asyncio.iscoroutine(failure.remedy):
+                                if asyncio.iscoroutine(failure.remedy):
                                     awaitable = failure.remedy
                                 else:
+                                    if asyncio.iscoroutinefunction(failure.remedy):
+                                        coro = failure.remedy
+                                    else:
 
-                                    async def awaitable() -> None:
-                                        result = failure.remedy()
-                                        if asyncio.iscoroutine(result):
-                                            await result
+                                        async def coro() -> None:
+                                            result = failure.remedy()
+                                            if asyncio.iscoroutine(result):
+                                                await result
 
-                                    _ = tg.create_task(awaitable)
-                                    servo.logger.info("💡 Attempting to apply remedy...")
+                                    awaitable = coro()
+
+                                await tg.create_task(awaitable)
+                                servo.logger.info("💡 Attempting to apply remedy...")
 
                         except asyncio.TimeoutError:
                             servo.logger.warning("💡 Remedy attempt timed out after 10s")

servo/connectors/kubernetes.py

@@ -1673,22 +1673,8 @@ class Config:
 
 
 def DNSSubdomainName(description: str | None = None) -> Any:
-    return pydantic.Field(
-        strip_whitespace=True,
-        min_length=1,
-        max_length=253,
-        regex="^[0-9a-zA-Z]([0-9a-zA-Z\\.-])*[0-9A-Za-z]$",
-        description=description,
-    )
-
-
-# DNSSubdomainName = pydantic.constr(
-#     strip_whitespace=True,
-#     min_length=1,
-#     max_length=253,
-#     regex="^[0-9a-zA-Z]([0-9a-zA-Z\\.-])*[0-9A-Za-z]$",
-# )
-DNSSubdomainName.__doc__ = """DNSSubdomainName models a Kubernetes DNS Subdomain Name used as the name for most resource types.
+    """DNSSubdomainName returns a pydantic.Field that models a Kubernetes DNS Subdomain Name used as the name for most
+    resource types. Its only parameter allows specifying a custom description for the field when the model schema is dumped.
 
     Valid DNS Subdomain Names conform to [RFC 1123](https://tools.ietf.org/html/rfc1123) and must:
         * contain no more than 253 characters
@@ -1698,6 +1684,14 @@ def DNSSubdomainName(description: str | None = None) -> Any:
 
     See https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names
     """
+    return pydantic.Field(
+        strip_whitespace=True,
+        min_length=1,
+        max_length=253,
+        regex="^[0-9a-zA-Z]([0-9a-zA-Z\\.-])*[0-9A-Za-z]$",
+        description=description,
+    )
+
 
 DNSLabelNameField = pydantic.Field(
     strip_whitespace=True,
@@ -1706,12 +1700,6 @@ def DNSSubdomainName(description: str | None = None) -> Any:
     regex="^[0-9a-zA-Z]([0-9a-zA-Z-])*[0-9A-Za-z]$",
 )
 DNSLabelName = Annotated[str, DNSLabelNameField]
-# DNSLabelName = pydantic.constr(
-#     strip_whitespace=True,
-#     min_length=1,
-#     max_length=63,
-#     regex="^[0-9a-zA-Z]([0-9a-zA-Z-])*[0-9A-Za-z]$",
-# )
 DNSLabelName.__doc__ = """DNSLabelName models a Kubernetes DNS Label Name identified used to name some resource types.
 
     Valid DNS Label Names conform to [RFC 1123](https://tools.ietf.org/html/rfc1123) and must:

servo/errors.py

@@ -136,9 +136,13 @@ def servo_error_from_group(
                 exc_list.append(exc)
 
         if top_error is None:
-            raise default_error(
-                message=str(exc_list[0]), additional_errors=exc_list
-            ) from exc_list[0]
+            try:
+                # raise from to capture full trace
+                raise default_error(
+                    message=str(exc_list[0]), additional_errors=exc_list
+                ) from exc_list[0]
+            except Exception as e:
+                return e
         else:
             top_error._additional_errors = exc_list
             return top_error
@@ -216,7 +220,7 @@ class EventAbortedError(EventError):
     """
 
 
-# Most errors have a default priority of 0
+# Higher numbers indicate a higher prioerity for the error. 0 is reserved for unrecognized exceptions
 _ERROR_PRIORITIES = {
     MeasurementFailedError: 1,
     AdjustmentFailedError: 2,

servo/servo.py

@@ -492,7 +492,7 @@ async def connect(self) -> None:
         )(self.try_connect)()
 
     async def try_connect(self) -> None:
-        self.logger.info("Saying HELLO.", end=" ")
+        self.logger.info("Saying HELLO.")
         try:
             await self.post_event(
                 servo.api.Events.hello,