You've already forked mariadb-columnstore-engine
							
							
				mirror of
				https://github.com/mariadb-corporation/mariadb-columnstore-engine.git
				synced 2025-11-03 17:13:17 +03:00 
			
		
		
		
	refactor: standardize error handling with new context managers for CMAPI exceptions
We do it for the API to return error details.
This commit is contained in:
		@@ -12,7 +12,7 @@ import cherrypy
 | 
			
		||||
import pyotp
 | 
			
		||||
import requests
 | 
			
		||||
 | 
			
		||||
from cmapi_server.exceptions import CMAPIBasicError
 | 
			
		||||
from cmapi_server.exceptions import CMAPIBasicError, cmapi_error_to_422
 | 
			
		||||
from cmapi_server.constants import (
 | 
			
		||||
    DEFAULT_MCS_CONF_PATH, DEFAULT_SM_CONF_PATH, EM_PATH_SUFFIX,
 | 
			
		||||
    MCS_BRM_CURRENT_PATH, MCS_EM_PATH, S3_BRM_CURRENT_PATH, SECRET_KEY,
 | 
			
		||||
@@ -924,14 +924,12 @@ class ClusterController:
 | 
			
		||||
        if node is None:
 | 
			
		||||
            raise_422_error(module_logger, func_name, 'missing node argument')
 | 
			
		||||
 | 
			
		||||
        try:
 | 
			
		||||
        with cmapi_error_to_422(module_logger, func_name):
 | 
			
		||||
            if not in_transaction:
 | 
			
		||||
                with TransactionManager(extra_nodes=[node]):
 | 
			
		||||
                    response = ClusterHandler.add_node(node, config)
 | 
			
		||||
            else:
 | 
			
		||||
                response = ClusterHandler.add_node(node, config)
 | 
			
		||||
        except CMAPIBasicError as err:
 | 
			
		||||
            raise_422_error(module_logger, func_name, err.message)
 | 
			
		||||
 | 
			
		||||
        module_logger.debug(f'{func_name} returns {str(response)}')
 | 
			
		||||
        return response
 | 
			
		||||
@@ -953,14 +951,12 @@ class ClusterController:
 | 
			
		||||
        if node is None:
 | 
			
		||||
            raise_422_error(module_logger, func_name, 'missing node argument')
 | 
			
		||||
 | 
			
		||||
        try:
 | 
			
		||||
        with cmapi_error_to_422(module_logger, func_name):
 | 
			
		||||
            if not in_transaction:
 | 
			
		||||
                with TransactionManager(remove_nodes=[node]):
 | 
			
		||||
                    response = ClusterHandler.remove_node(node, config)
 | 
			
		||||
            else:
 | 
			
		||||
                response = ClusterHandler.remove_node(node, config)
 | 
			
		||||
        except CMAPIBasicError as err:
 | 
			
		||||
            raise_422_error(module_logger, func_name, err.message)
 | 
			
		||||
 | 
			
		||||
        module_logger.debug(f'{func_name} returns {str(response)}')
 | 
			
		||||
        return response
 | 
			
		||||
@@ -1079,10 +1075,8 @@ class ClusterController:
 | 
			
		||||
                module_logger, func_name, 'Wrong verification key.'
 | 
			
		||||
            )
 | 
			
		||||
 | 
			
		||||
        try:
 | 
			
		||||
        with cmapi_error_to_422(module_logger, func_name):
 | 
			
		||||
            response = ClusterHandler.set_api_key(new_api_key, totp_key)
 | 
			
		||||
        except CMAPIBasicError as err:
 | 
			
		||||
            raise_422_error(module_logger, func_name, err.message)
 | 
			
		||||
 | 
			
		||||
        module_logger.debug(f'{func_name} returns {str(response)}')
 | 
			
		||||
        return response
 | 
			
		||||
 
 | 
			
		||||
@@ -1,5 +1,11 @@
 | 
			
		||||
"""Module contains custom exceptions."""
 | 
			
		||||
 | 
			
		||||
from collections.abc import Iterator
 | 
			
		||||
from contextlib import contextmanager
 | 
			
		||||
from typing import Optional
 | 
			
		||||
 | 
			
		||||
from cmapi_server.controllers.error import APIError
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
class CMAPIBasicError(Exception):
 | 
			
		||||
    """Basic exception raised for CMAPI related processes.
 | 
			
		||||
@@ -20,3 +26,35 @@ class CEJError(CMAPIBasicError):
 | 
			
		||||
    Attributes:
 | 
			
		||||
        message -- explanation of the error
 | 
			
		||||
    """
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
@contextmanager
 | 
			
		||||
def exc_to_cmapi_error(prefix: Optional[str] = None) -> Iterator[None]:
 | 
			
		||||
    """Context manager to standardize error wrapping into CMAPIBasicError.
 | 
			
		||||
 | 
			
		||||
    Re-raises existing CMAPIBasicError untouched (to preserve detailed
 | 
			
		||||
    messages). Any other exception type is wrapped into CMAPIBasicError with an
 | 
			
		||||
    optional prefix and the original exception string appended as details.
 | 
			
		||||
 | 
			
		||||
    :param prefix: Optional message prefix for wrapped errors
 | 
			
		||||
    :raises CMAPIBasicError: for any wrapped non-CMAPIBasicError exceptions
 | 
			
		||||
    """
 | 
			
		||||
    try:
 | 
			
		||||
        yield
 | 
			
		||||
    except CMAPIBasicError:
 | 
			
		||||
        # Preserve detailed messages from deeper layers (e.g., validation)
 | 
			
		||||
        raise
 | 
			
		||||
    except Exception as err:
 | 
			
		||||
        msg = f"{prefix}. Details: {err}" if prefix else str(err)
 | 
			
		||||
        raise CMAPIBasicError(msg) from err
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
@contextmanager
 | 
			
		||||
def cmapi_error_to_422(logger, func_name: str) -> Iterator[None]:
 | 
			
		||||
    """Convert CMAPIBasicError to HTTP 422 APIError."""
 | 
			
		||||
    try:
 | 
			
		||||
        yield
 | 
			
		||||
    except CMAPIBasicError as err:
 | 
			
		||||
        # mirror raise_422_error behavior locally to avoid circular imports
 | 
			
		||||
        logger.error(f'{func_name} {err.message}', exc_info=False)
 | 
			
		||||
        raise APIError(422, err.message) from err
 | 
			
		||||
 
 | 
			
		||||
@@ -6,12 +6,13 @@ from typing import Optional
 | 
			
		||||
 | 
			
		||||
from mcs_node_control.models.misc import get_dbrm_master
 | 
			
		||||
from mcs_node_control.models.node_config import NodeConfig
 | 
			
		||||
from tracing.traced_session import get_traced_session
 | 
			
		||||
 | 
			
		||||
from cmapi_server.constants import (
 | 
			
		||||
    CMAPI_CONF_PATH,
 | 
			
		||||
    DEFAULT_MCS_CONF_PATH,
 | 
			
		||||
)
 | 
			
		||||
from cmapi_server.exceptions import CMAPIBasicError
 | 
			
		||||
from cmapi_server.exceptions import CMAPIBasicError, exc_to_cmapi_error
 | 
			
		||||
from cmapi_server.helpers import (
 | 
			
		||||
    broadcast_new_config,
 | 
			
		||||
    get_active_nodes,
 | 
			
		||||
@@ -27,7 +28,6 @@ from cmapi_server.node_manipulation import (
 | 
			
		||||
    remove_node,
 | 
			
		||||
    switch_node_maintenance,
 | 
			
		||||
)
 | 
			
		||||
from tracing.traced_session import get_traced_session
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
class ClusterAction(Enum):
 | 
			
		||||
@@ -171,7 +171,7 @@ class ClusterHandler:
 | 
			
		||||
 | 
			
		||||
        response = {'timestamp': str(datetime.now())}
 | 
			
		||||
 | 
			
		||||
        try:
 | 
			
		||||
        with exc_to_cmapi_error(prefix='Error while adding node'):
 | 
			
		||||
            add_node(
 | 
			
		||||
                node, input_config_filename=config,
 | 
			
		||||
                output_config_filename=config
 | 
			
		||||
@@ -181,8 +181,6 @@ class ClusterHandler:
 | 
			
		||||
                    host=node, input_config_filename=config,
 | 
			
		||||
                    output_config_filename=config
 | 
			
		||||
                )
 | 
			
		||||
        except Exception as err:
 | 
			
		||||
            raise CMAPIBasicError('Error while adding node.') from err
 | 
			
		||||
 | 
			
		||||
        response['node_id'] = node
 | 
			
		||||
        update_revision_and_manager(
 | 
			
		||||
@@ -218,13 +216,11 @@ class ClusterHandler:
 | 
			
		||||
        )
 | 
			
		||||
        response = {'timestamp': str(datetime.now())}
 | 
			
		||||
 | 
			
		||||
        try:
 | 
			
		||||
        with exc_to_cmapi_error(prefix='Error while removing node'):
 | 
			
		||||
            remove_node(
 | 
			
		||||
                node, input_config_filename=config,
 | 
			
		||||
                output_config_filename=config
 | 
			
		||||
            )
 | 
			
		||||
        except Exception as err:
 | 
			
		||||
            raise CMAPIBasicError('Error while removing node.') from err
 | 
			
		||||
 | 
			
		||||
        response['node_id'] = node
 | 
			
		||||
        active_nodes = get_active_nodes(config)
 | 
			
		||||
 
 | 
			
		||||
@@ -15,6 +15,7 @@ from typing import Optional
 | 
			
		||||
import requests
 | 
			
		||||
from lxml import etree
 | 
			
		||||
from mcs_node_control.models.node_config import NodeConfig
 | 
			
		||||
from tracing.traced_session import get_traced_session
 | 
			
		||||
 | 
			
		||||
from cmapi_server import helpers
 | 
			
		||||
from cmapi_server.constants import (
 | 
			
		||||
@@ -25,7 +26,6 @@ from cmapi_server.constants import (
 | 
			
		||||
    MCS_DATA_PATH,
 | 
			
		||||
)
 | 
			
		||||
from cmapi_server.managers.network import NetworkManager
 | 
			
		||||
from tracing.traced_session import get_traced_session
 | 
			
		||||
 | 
			
		||||
PMS_NODE_PORT = '8620'
 | 
			
		||||
EXEMGR_NODE_PORT = '8601'
 | 
			
		||||
@@ -640,7 +640,7 @@ def _rebalance_dbroots(root, test_mode=False):
 | 
			
		||||
                        # timed out
 | 
			
		||||
                        # possible node is not ready, leave retry as-is
 | 
			
		||||
                        pass
 | 
			
		||||
                    except Exception as e:
 | 
			
		||||
                    except Exception:
 | 
			
		||||
                        retry = False
 | 
			
		||||
 | 
			
		||||
                if not found_master:
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user