SK-2954: Split ISkyflow into BaseSkyflow/BaseSkyflowImpl, make connection/detect structurally absent when unsupported

Renames ISkyflow -> BaseSkyflow (pure interface) and the old BaseSkyflow ->
BaseSkyflowImpl (concrete). Moves connection/detect support into
ConnectionCapable/DetectCapable interfaces + ConnectionMixin/DetectMixin in a
new common/client/utils/_utils.py, conditionally composed into a variant's
Skyflow class by make_skyflow_class() so unsupported variants (e.g. flowvault)
genuinely lack .connection()/.detect() (AttributeError) instead of raising
NotImplementedError from a present-but-guarded method.

Also fixes two review-flagged bugs: adding a vault/connection config with a
duplicate id to an already-built client now raises SkyflowError instead of
silently overwriting the existing entry, and update_connection_config no
longer risks a bare KeyError on a missing connection_id. Extracts the
Builder's raw NotImplementedError string literals into named constants.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: saileshwar-skyflow

common/client/base_skyflow.py

@@ -1,21 +1,19 @@
 from abc import ABC, abstractmethod
 from collections import OrderedDict
-from functools import partial
-
 from common.errors import SkyflowError
 from common.utils import SkyflowMessages
-from common.utils.enums import LogLevel as _CommonLogLevel
-from common.utils.logger import Logger as _CommonLogger, log_info, log_warn
+from common.utils.logger import log_info, log_warn
 from common.utils.constants import OptionField
-from common.utils.validations import (
-    validate_vault_config as _common_validate_vault_config,
-    validate_update_vault_config as _common_validate_update_vault_config,
-    validate_log_level as _common_validate_log_level,
-    validate_credentials as _common_validate_credentials,
+
+_BUILDER_TEMPLATE_ERROR = (
+    "BaseSkyflowImpl.Builder is an interface template -- build a concrete Skyflow "
+    "class via make_skyflow_class() instead of using it directly. Missing: {missing}"
 )
+_CONNECTIONS_NOT_SUPPORTED_ERROR = "Connections are not supported by this Skyflow SDK variant"
+_DETECT_NOT_SUPPORTED_ERROR = "Detect is not supported by this Skyflow SDK variant"
 
 
-class ISkyflow(ABC):
+class BaseSkyflow(ABC):
     @classmethod
     @abstractmethod
     def builder(cls):
@@ -37,22 +35,6 @@ def update_vault_config(self, config):
     def get_vault_config(self, vault_id):
         raise NotImplementedError
 
-    @abstractmethod
-    def add_connection_config(self, config):
-        raise NotImplementedError
-
-    @abstractmethod
-    def remove_connection_config(self, connection_id):
-        raise NotImplementedError
-
-    @abstractmethod
-    def update_connection_config(self, config):
-        raise NotImplementedError
-
-    @abstractmethod
-    def get_connection_config(self, connection_id):
-        raise NotImplementedError
-
     @abstractmethod
     def add_skyflow_credentials(self, credentials):
         raise NotImplementedError
@@ -77,19 +59,11 @@ def get_log_level(self):
     def vault(self, vault_id=None):
         raise NotImplementedError
 
-    @abstractmethod
-    def connection(self, connection_id=None):
-        raise NotImplementedError
 
-    @abstractmethod
-    def detect(self, vault_id=None):
-        raise NotImplementedError
-
-
-class BaseSkyflow(ISkyflow):
+class BaseSkyflowImpl(BaseSkyflow):
 
     def __init__(self, builder):
-        if type(self) is BaseSkyflow:
+        if type(self) is BaseSkyflowImpl:
             raise SkyflowError(
                 SkyflowMessages.Error.BASE_SKYFLOW_INSTANTIATION_NOT_ALLOWED.value,
                 SkyflowMessages.ErrorCodes.INVALID_INPUT.value,
@@ -102,7 +76,7 @@ def builder(cls):
         return cls.Builder()
 
     def add_vault_config(self, config):
-        self.__builder._Builder__add_vault_config(config)
+        self.__builder._add_vault_config(config)
         return self
 
     def remove_vault_config(self, vault_id):
@@ -114,34 +88,15 @@ def update_vault_config(self, config):
     def get_vault_config(self, vault_id):
         return self.__builder.get_vault_config(vault_id).get(OptionField.VAULT_CLIENT).get_config()
 
-    def add_connection_config(self, config):
-        self.__builder._require_connections()
-        self.__builder._Builder__add_connection_config(config)
-        return self
-
-    def remove_connection_config(self, connection_id):
-        self.__builder._require_connections()
-        self.__builder.remove_connection_config(connection_id)
-        return self
-
-    def update_connection_config(self, config):
-        self.__builder._require_connections()
-        self.__builder.update_connection_config(config)
-        return self
-
-    def get_connection_config(self, connection_id):
-        self.__builder._require_connections()
-        return self.__builder.get_connection_config(connection_id).get(OptionField.VAULT_CLIENT).get_config()
-
     def add_skyflow_credentials(self, credentials):
-        self.__builder._Builder__add_skyflow_credentials(credentials)
+        self.__builder._add_skyflow_credentials(credentials)
         return self
 
     def update_skyflow_credentials(self, credentials):
-        self.__builder._Builder__add_skyflow_credentials(credentials)
+        self.__builder._add_skyflow_credentials(credentials)
 
     def set_log_level(self, log_level):
-        self.__builder._Builder__set_log_level(log_level)
+        self.__builder._set_log_level(log_level)
         return self
 
     def update_log_level(self, log_level):
@@ -150,21 +105,14 @@ def update_log_level(self, log_level):
         return self.set_log_level(log_level)
 
     def get_log_level(self):
-        return self.__builder._Builder__log_level
+        return self.__builder.get_log_level()
 
     def vault(self, vault_id=None):
         vault_config = self.__builder.get_vault_config(vault_id)
         return vault_config.get(OptionField.VAULT_CONTROLLER)
 
-    def connection(self, connection_id=None):
-        self.__builder._require_connections()
-        connection_config = self.__builder.get_connection_config(connection_id)
-        return connection_config.get(OptionField.CONTROLLER)
-
-    def detect(self, vault_id=None):
-        self.__builder._require_detect()
-        vault_config = self.__builder.get_vault_config(vault_id)
-        return vault_config.get(OptionField.DETECT_CONTROLLER)
+    def _get_builder(self):
+        return self.__builder
 
     class Builder(ABC):
         _vault_client_cls = None
@@ -192,10 +140,7 @@ class Builder(ABC):
         def __init__(self):
             missing = [hook for hook in self._REQUIRED_HOOKS if getattr(self, hook) is None]
             if missing:
-                raise NotImplementedError(
-                    "BaseSkyflow.Builder is an interface template -- build a concrete Skyflow "
-                    f"class via make_skyflow_class() instead of using it directly. Missing: {', '.join(missing)}"
-                )
+                raise NotImplementedError(_BUILDER_TEMPLATE_ERROR.format(missing=', '.join(missing)))
             self.__vault_configs = OrderedDict()
             self.__vault_list = list()
             self.__connection_configs = OrderedDict()
@@ -206,11 +151,11 @@ def __init__(self):
 
         def _require_connections(self):
             if self._connection_cls is None:
-                raise NotImplementedError("Connections are not supported by this Skyflow SDK variant")
+                raise NotImplementedError(_CONNECTIONS_NOT_SUPPORTED_ERROR)
 
         def _require_detect(self):
             if self._detect_cls is None:
-                raise NotImplementedError("Detect is not supported by this Skyflow SDK variant")
+                raise NotImplementedError(_DETECT_NOT_SUPPORTED_ERROR)
 
         def add_vault_config(self, config):
             vault_id = config.get(OptionField.VAULT_ID)
@@ -282,7 +227,7 @@ def remove_connection_config(self, connection_id):
         def update_connection_config(self, config):
             self._require_connections()
             self._validate_update_connection_config(self.__logger, config)
-            connection_id = config[OptionField.CONNECTION_ID]
+            connection_id = config.get(OptionField.CONNECTION_ID)
             if connection_id not in self.__connection_configs:
                 raise SkyflowError(self._skyflow_messages.Error.CONNECTION_ID_NOT_IN_CONFIG_LIST.value.format(connection_id), self._skyflow_messages.ErrorCodes.INVALID_INPUT.value)
             connection_config = self.__connection_configs[connection_id]
@@ -312,9 +257,17 @@ def set_log_level(self, log_level):
         def get_logger(self):
             return self.__logger
 
-        def __add_vault_config(self, config):
+        def get_log_level(self):
+            return self.__log_level
+
+        def _add_vault_config(self, config):
             self._validate_vault_config(self.__logger, config)
             vault_id = config.get(OptionField.VAULT_ID)
+            if vault_id in self.__vault_configs:
+                raise SkyflowError(
+                    self._skyflow_messages.Error.VAULT_ID_ALREADY_EXISTS.value.format(vault_id),
+                    self._skyflow_messages.ErrorCodes.INVALID_INPUT.value
+                )
             vault_client = self._vault_client_cls(config)
             vault_config = {
                 OptionField.VAULT_CLIENT: vault_client,
@@ -327,34 +280,39 @@ def __add_vault_config(self, config):
             if self._detect_cls is not None:
                 log_info(self._skyflow_messages.Info.DETECT_CONTROLLER_INITIALIZED.value.format(vault_id), self.__logger)
 
-        def __add_connection_config(self, config):
+        def _add_connection_config(self, config):
             self._validate_connection_config(self.__logger, config)
             connection_id = config.get(OptionField.CONNECTION_ID)
+            if connection_id in self.__connection_configs:
+                raise SkyflowError(
+                    self._skyflow_messages.Error.CONNECTION_ID_ALREADY_EXISTS.value.format(connection_id),
+                    self._skyflow_messages.ErrorCodes.INVALID_INPUT.value
+                )
             vault_client = self._vault_client_cls(config)
             self.__connection_configs[connection_id] = {
                 OptionField.VAULT_CLIENT: vault_client,
                 OptionField.CONTROLLER: self._connection_cls(vault_client)
             }
             log_info(self._skyflow_messages.Info.CONNECTION_CONTROLLER_INITIALIZED.value.format(connection_id), self.__logger)
 
-        def __update_vault_client_logger(self, log_level, logger):
+        def _update_vault_client_logger(self, log_level, logger):
             for vault_id, vault_config in self.__vault_configs.items():
                 vault_config.get(OptionField.VAULT_CLIENT).set_logger(log_level, logger)
 
             for connection_id, connection_config in self.__connection_configs.items():
                 connection_config.get(OptionField.VAULT_CLIENT).set_logger(log_level, logger)
 
-        def __set_log_level(self, log_level):
+        def _set_log_level(self, log_level):
             self._validate_log_level(self.__logger, log_level)
             self.__log_level = log_level
             self.__logger.set_log_level(log_level)
             if self._set_active_log_level is not None:
                 self._set_active_log_level(log_level)
-            self.__update_vault_client_logger(log_level, self.__logger)
+            self._update_vault_client_logger(log_level, self.__logger)
             log_info(self._skyflow_messages.Info.LOGGER_SETUP_DONE.value, self.__logger)
             log_info(self._skyflow_messages.Info.CURRENT_LOG_LEVEL.value.format(self.__log_level), self.__logger)
 
-        def __add_skyflow_credentials(self, credentials):
+        def _add_skyflow_credentials(self, credentials):
             if credentials is not None:
                 self.__skyflow_credentials = credentials
                 self._validate_credentials(self.__logger, credentials)
@@ -371,51 +329,13 @@ def build(self):
                 self._set_active_log_level(self.__log_level)
 
             for config in self.__vault_list:
-                self.__add_vault_config(config)
+                self._add_vault_config(config)
 
             for config in self.__connection_list:
-                self.__add_connection_config(config)
+                self._add_connection_config(config)
 
-            self.__update_vault_client_logger(self.__log_level, self.__logger)
+            self._update_vault_client_logger(self.__log_level, self.__logger)
 
-            self.__add_skyflow_credentials(self.__skyflow_credentials)
+            self._add_skyflow_credentials(self.__skyflow_credentials)
 
             return self._skyflow_cls(self)
-
-
-def make_skyflow_class(*, vault_client_cls, vault_controller_cls, skyflow_messages,
-                        validate_vault_config=None, validate_update_vault_config=None,
-                        validate_log_level=None, validate_credentials=None,
-                        logger_cls=_CommonLogger, default_log_level=_CommonLogLevel.ERROR,
-                        connection_cls=None, detect_cls=None,
-                        validate_connection_config=None, validate_update_connection_config=None,
-                        set_active_log_level=None):
-
-    if connection_cls is not None and (validate_connection_config is None or validate_update_connection_config is None):
-        raise ValueError("connection_cls requires validate_connection_config and validate_update_connection_config")
-
-    validate_vault_config = validate_vault_config or partial(_common_validate_vault_config, messages=skyflow_messages)
-    validate_update_vault_config = validate_update_vault_config or partial(_common_validate_update_vault_config, messages=skyflow_messages)
-    validate_log_level = validate_log_level or partial(_common_validate_log_level, messages=skyflow_messages)
-    validate_credentials = validate_credentials or partial(_common_validate_credentials, messages=skyflow_messages)
-
-    builder_attrs = {
-        '_vault_client_cls': vault_client_cls,
-        '_vault_controller_cls': vault_controller_cls,
-        '_connection_cls': connection_cls,
-        '_detect_cls': detect_cls,
-        '_logger_cls': logger_cls,
-        '_default_log_level': default_log_level,
-        '_skyflow_messages': skyflow_messages,
-        '_validate_vault_config': staticmethod(validate_vault_config),
-        '_validate_update_vault_config': staticmethod(validate_update_vault_config),
-        '_validate_connection_config': staticmethod(validate_connection_config) if validate_connection_config else None,
-        '_validate_update_connection_config': staticmethod(validate_update_connection_config) if validate_update_connection_config else None,
-        '_validate_log_level': staticmethod(validate_log_level),
-        '_validate_credentials': staticmethod(validate_credentials),
-        '_set_active_log_level': staticmethod(set_active_log_level) if set_active_log_level else None,
-    }
-    variant_builder = type('Builder', (BaseSkyflow.Builder,), builder_attrs)
-    variant_skyflow = type('Skyflow', (BaseSkyflow,), {'Builder': variant_builder})
-    variant_builder._skyflow_cls = variant_skyflow
-    return variant_skyflow

common/client/utils/__init__.py

@@ -0,0 +1 @@
+from common.client.utils._utils import ConnectionCapable, DetectCapable, ConnectionMixin, DetectMixin, make_skyflow_class