From 5e54e1476c9f29fbe3d4f4293bfa600179e76343 Mon Sep 17 00:00:00 2001 From: lilyz-ai Date: Tue, 31 Mar 2026 23:46:13 +0000 Subject: [PATCH 1/3] fix(gcp): surface GCP Secret Manager errors and validate DB credentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously get_key_file silently returned {} on any error, causing str(None) → 'None' to be used as the port, leading to a cryptic 'invalid literal for int() with base 10: None' error from SQLAlchemy. - Remove the try/except in get_key_file so GCP errors surface directly - Validate required fields after fetching creds and raise with a clear message - Use dict key access instead of .get() for required fields --- .../model_engine_server/core/gcp/secrets.py | 9 ++------ model-engine/model_engine_server/db/base.py | 23 +++++++++++++++---- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/model-engine/model_engine_server/core/gcp/secrets.py b/model-engine/model_engine_server/core/gcp/secrets.py index 739a25bd..431237d7 100644 --- a/model-engine/model_engine_server/core/gcp/secrets.py +++ b/model-engine/model_engine_server/core/gcp/secrets.py @@ -23,10 +23,5 @@ def get_key_file(secret_name: str, gcp_project: Optional[str] = None): if gcp_project is not None: secret_name = f"projects/{gcp_project}/secrets/{secret_name}/versions/latest" client = secretmanager.SecretManagerServiceClient() - try: - response = client.access_secret_version(name=secret_name) - return json.loads(response.payload.data.decode("utf-8")) - except Exception as e: - logger.error(e) - logger.error(f"Failed to retrieve secret: {secret_name}") - return {} + response = client.access_secret_version(name=secret_name) + return json.loads(response.payload.data.decode("utf-8")) diff --git a/model-engine/model_engine_server/db/base.py b/model-engine/model_engine_server/db/base.py index f03c293a..2d11f55b 100644 --- a/model-engine/model_engine_server/db/base.py +++ b/model-engine/model_engine_server/db/base.py @@ -76,11 +76,24 @@ def get_engine_url( db_secret_gcp_project_id = os.environ.get("DB_SECRET_GCP_PROJECT_ID") creds = get_gcp_key_file(db_secret_name, db_secret_gcp_project_id) - user = creds.get("username") - password = creds.get("password") - host = creds.get("clusterHostRo") if read_only else creds.get("clusterHost") - port = str(creds.get("port")) - dbname = creds.get("dbname") + missing = [ + k + for k in ("username", "password", "clusterHost", "port", "dbname") + if not creds.get(k) + ] + if missing: + raise ValueError( + f"GCP DB secret {db_secret_name!r} is missing required fields: {missing}" + ) + user = creds["username"] + password = creds["password"] + host = ( + creds.get("clusterHostRo") or creds["clusterHost"] + if read_only + else creds["clusterHost"] + ) + port = str(creds["port"]) + dbname = creds["dbname"] else: user = os.environ.get("DB_USER", "postgres") password = os.environ.get("DB_PASSWORD", "postgres") From 99bdec3fed0e0d2c8a54eff55921f51c73ee939a Mon Sep 17 00:00:00 2001 From: lilyz-ai Date: Tue, 31 Mar 2026 23:54:28 +0000 Subject: [PATCH 2/3] fix(plugins): only swallow ModuleNotFoundError when plugins itself is missing The bare 'except ModuleNotFoundError' was catching any ModuleNotFoundError raised inside the plugins module (e.g. a missing dependency), silently falling back to defaults and masking the real error. Check e.name to only fall back when 'plugins' itself is not installed. --- .../model_engine_server/api/dependencies.py | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/model-engine/model_engine_server/api/dependencies.py b/model-engine/model_engine_server/api/dependencies.py index 7c245197..b5737329 100644 --- a/model-engine/model_engine_server/api/dependencies.py +++ b/model-engine/model_engine_server/api/dependencies.py @@ -202,10 +202,10 @@ def get_monitoring_metrics_gateway() -> MonitoringMetricsGateway: ) return get_custom_monitoring_metrics_gateway() - except ModuleNotFoundError: + except ModuleNotFoundError as e: + if e.name is None or not e.name.startswith("plugins"): + raise return get_default_monitoring_metrics_gateway() - finally: - pass def _get_external_interfaces( @@ -453,7 +453,9 @@ async def get_external_interfaces(): from plugins.dependencies import get_external_interfaces as get_custom_external_interfaces ei = get_custom_external_interfaces() - except ModuleNotFoundError: + except ModuleNotFoundError as e: + if e.name is None or not e.name.startswith("plugins"): + raise ei = get_default_external_interfaces() try: yield ei @@ -468,7 +470,9 @@ async def get_external_interfaces_read_only(): ) ei = get_custom_external_interfaces_read_only() - except ModuleNotFoundError: + except ModuleNotFoundError as e: + if e.name is None or not e.name.startswith("plugins"): + raise ei = get_default_external_interfaces_read_only() try: yield ei @@ -489,10 +493,10 @@ async def get_auth_repository(): from plugins.dependencies import get_auth_repository as get_custom_auth_repository yield get_custom_auth_repository() - except ModuleNotFoundError: + except ModuleNotFoundError as e: + if e.name is None or not e.name.startswith("plugins"): + raise yield get_default_auth_repository() - finally: - pass async def verify_authentication( From 5c282dcdf41d8f571d0165af5e15ab8a61b0d637 Mon Sep 17 00:00:00 2001 From: lilyz-ai Date: Wed, 1 Apr 2026 00:01:20 +0000 Subject: [PATCH 3/3] fix(plugins): log warning when falling back due to missing plugins module --- model-engine/model_engine_server/api/dependencies.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/model-engine/model_engine_server/api/dependencies.py b/model-engine/model_engine_server/api/dependencies.py index b5737329..d8500f5d 100644 --- a/model-engine/model_engine_server/api/dependencies.py +++ b/model-engine/model_engine_server/api/dependencies.py @@ -205,6 +205,9 @@ def get_monitoring_metrics_gateway() -> MonitoringMetricsGateway: except ModuleNotFoundError as e: if e.name is None or not e.name.startswith("plugins"): raise + logger.warning( + "plugins module not found, falling back to default monitoring metrics gateway" + ) return get_default_monitoring_metrics_gateway() @@ -456,6 +459,7 @@ async def get_external_interfaces(): except ModuleNotFoundError as e: if e.name is None or not e.name.startswith("plugins"): raise + logger.warning("plugins module not found, falling back to default external interfaces") ei = get_default_external_interfaces() try: yield ei @@ -473,6 +477,9 @@ async def get_external_interfaces_read_only(): except ModuleNotFoundError as e: if e.name is None or not e.name.startswith("plugins"): raise + logger.warning( + "plugins module not found, falling back to default external interfaces (read-only)" + ) ei = get_default_external_interfaces_read_only() try: yield ei @@ -496,6 +503,7 @@ async def get_auth_repository(): except ModuleNotFoundError as e: if e.name is None or not e.name.startswith("plugins"): raise + logger.warning("plugins module not found, falling back to default auth repository") yield get_default_auth_repository()