From 94eb3b0ea90bba7b43b0ca6e0f29d39387df3b85 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 23 Mar 2026 09:24:56 +0530 Subject: [PATCH 1/8] credentials: http error and documentation --- backend/app/api/docs/credentials/create.md | 43 +++++++++++++++ backend/app/api/docs/onboarding/onboarding.md | 18 ++++++- backend/app/core/providers.py | 10 ++-- backend/app/crud/credentials.py | 36 ++++++++++--- backend/app/models/onboarding.py | 53 ++++++++----------- backend/app/utils.py | 13 +++-- 6 files changed, 129 insertions(+), 44 deletions(-) diff --git a/backend/app/api/docs/credentials/create.md b/backend/app/api/docs/credentials/create.md index 139a3c85c..84c9eeeab 100644 --- a/backend/app/api/docs/credentials/create.md +++ b/backend/app/api/docs/credentials/create.md @@ -1,3 +1,46 @@ Persist new credentials for the current organization and project. Credentials are encrypted and stored securely for provider integrations (OpenAI, Langfuse, etc.). Only one credential per provider is allowed per organization-project combination. + +### Supported Providers: +- **LLM:** openai, sarvamai, google(gemini) +- **Observability:** langfuse +- **Audio:** elevenlabs + +### Examples: + +#### Single Provider +```json +{ + "credential": { + "openai": { + "api_key": "sk-proj-..." + } + } +} +``` + +#### Multiple Providers +```json +{ + "credential": { + "openai": { + "api_key": "sk-proj-..." + }, + "google": { + "api_key": "AIzaSy..." + }, + "sarvamai": { + "api_key": "sarvam-..." + }, + "elevenlabs": { + "api_key": "sk_..." + }, + "langfuse": { + "public_key": "pk-lf-...", + "secret_key": "sk-lf-...", + "host": "https://cloud.langfuse.com" + } + } +} +``` diff --git a/backend/app/api/docs/onboarding/onboarding.md b/backend/app/api/docs/onboarding/onboarding.md index aab8dc8e3..08c1e6516 100644 --- a/backend/app/api/docs/onboarding/onboarding.md +++ b/backend/app/api/docs/onboarding/onboarding.md @@ -30,8 +30,9 @@ - We’ve also included a list of the providers currently supported by kaapi. ### Supported Providers - - **LLM:** openai + - **LLM:** openai, google, sarvamai - **Observability:** langfuse + - **Audio:** elevenlabs ### Example: For sending multiple credentials - ``` @@ -41,6 +42,21 @@ "api_key": "sk-proj-..." } }, + { + "google": { + "api_key": "AIzaSy..." + } + }, + { + "sarvamai": { + "api_key": "sarvam-..." + } + }, + { + "elevenlabs": { + "api_key": "sk_..." + } + }, { "langfuse": { "public_key": "pk-lf-...", diff --git a/backend/app/core/providers.py b/backend/app/core/providers.py index 980ef4164..33f687c85 100644 --- a/backend/app/core/providers.py +++ b/backend/app/core/providers.py @@ -10,13 +10,15 @@ class Provider(str, Enum): """Enumeration of supported credential providers.""" OPENAI = "openai" - AWS = "aws" LANGFUSE = "langfuse" GOOGLE = "google" SARVAMAI = "sarvamai" ELEVENLABS = "elevenlabs" +# AWS = "aws" + + @dataclass class ProviderConfig: """Configuration for a provider including its required credential fields.""" @@ -27,15 +29,15 @@ class ProviderConfig: # Provider configurations PROVIDER_CONFIGS: Dict[Provider, ProviderConfig] = { Provider.OPENAI: ProviderConfig(required_fields=["api_key"]), - Provider.AWS: ProviderConfig( - required_fields=["access_key_id", "secret_access_key", "region"] - ), Provider.LANGFUSE: ProviderConfig( required_fields=["secret_key", "public_key", "host"] ), Provider.GOOGLE: ProviderConfig(required_fields=["api_key"]), Provider.SARVAMAI: ProviderConfig(required_fields=["api_key"]), Provider.ELEVENLABS: ProviderConfig(required_fields=["api_key"]), + # Provider.AWS: ProviderConfig( + # required_fields=["access_key_id", "secret_access_key", "region"] + # ), } diff --git a/backend/app/crud/credentials.py b/backend/app/crud/credentials.py index f430ed71b..d46e94e4d 100644 --- a/backend/app/crud/credentials.py +++ b/backend/app/crud/credentials.py @@ -28,8 +28,14 @@ def set_creds_for_org( for provider, credentials in creds_add.credential.items(): # Validate provider and credentials - validate_provider(provider) - validate_provider_credentials(provider, credentials) + try: + validate_provider(provider) + validate_provider_credentials(provider, credentials) + except ValueError as e: + logger.error( + f"[set_creds_for_org] Validation error | project_id: {project_id}, provider: {provider}, error: {str(e)}" + ) + raise HTTPException(status_code=400, detail=str(e)) # Encrypt entire credentials object encrypted_credentials = encrypt_credentials(credentials) @@ -144,7 +150,13 @@ def get_provider_credential( Raises: HTTPException: If credentials are not found """ - validate_provider(provider) + try: + validate_provider(provider) + except ValueError as e: + logger.error( + f"[get_provider_credential] Validation error | organization_id: {org_id}, project_id: {project_id}, provider: {provider}, error: {str(e)}" + ) + raise HTTPException(status_code=400, detail=str(e)) statement = select(Credential).where( Credential.organization_id == org_id, @@ -176,8 +188,14 @@ def update_creds_for_org( if not creds_in.provider or not creds_in.credential: raise ValueError("Provider and credential must be provided") - validate_provider(creds_in.provider) - validate_provider_credentials(creds_in.provider, creds_in.credential) + try: + validate_provider(creds_in.provider) + validate_provider_credentials(creds_in.provider, creds_in.credential) + except ValueError as e: + logger.error( + f"[update_creds_for_org] Validation error | organization_id: {org_id}, project_id: {project_id}, provider: {creds_in.provider}, error: {str(e)}" + ) + raise HTTPException(status_code=400, detail=str(e)) # Encrypt the entire credentials object encrypted_credentials = encrypt_credentials(creds_in.credential) @@ -216,7 +234,13 @@ def remove_provider_credential( Raises: HTTPException: If credentials not found or deletion fails """ - validate_provider(provider) + try: + validate_provider(provider) + except ValueError as e: + logger.error( + f"[remove_provider_credential] Validation error | organization_id: {org_id}, project_id: {project_id}, provider: {provider}, error: {str(e)}" + ) + raise HTTPException(status_code=400, detail=str(e)) # Verify credentials exist before attempting delete creds = get_provider_credential( diff --git a/backend/app/models/onboarding.py b/backend/app/models/onboarding.py index 43b588b98..794f9521d 100644 --- a/backend/app/models/onboarding.py +++ b/backend/app/models/onboarding.py @@ -93,36 +93,29 @@ def _validate_credential_list(cls, v: list[dict[str, dict[str, str]]] | None): "credential must be a list of single-key dicts (e.g., {'openai': {...}})." ) - errors: list[str] = [] - - for idx, item in enumerate(v): - try: - if not isinstance(item, dict): - raise TypeError( - "must be a dict with a single provider key like {'openai': {...}}." - ) - if len(item) != 1: - raise ValueError( - "must have exactly one provider key like {'openai': {...}}." - ) - - (provider_key,) = item.keys() - values = item[provider_key] - - validate_provider(provider_key) - - if not isinstance(values, dict): - raise TypeError( - f"value for provider '{provider_key}' must be an object/dict." - ) - - validate_provider_credentials(provider_key, values) - - except (TypeError, ValueError) as e: - errors.append(f"[{idx}] {e}") - - if errors: - raise ValueError("credential validation failed:\n" + "\n".join(errors)) + for item in v: + if not isinstance(item, dict): + raise TypeError( + "Credential must be a dict with a single provider key like {'openai': {...}}." + ) + if len(item) != 1: + raise ValueError( + "Credential must have exactly one provider key like {'openai': {...}}." + ) + + (provider_key,) = item.keys() + values = item[provider_key] + + # validate_provider will raise ValueError with clear message if invalid + validate_provider(provider_key) + + if not isinstance(values, dict): + raise TypeError( + f"Value for provider '{provider_key}' must be an object/dict." + ) + + # validate_provider_credentials will raise ValueError with clear message if invalid + validate_provider_credentials(provider_key, values) return v diff --git a/backend/app/utils.py b/backend/app/utils.py index d576468f6..1aa9edf39 100644 --- a/backend/app/utils.py +++ b/backend/app/utils.py @@ -75,10 +75,17 @@ def failure_response( loc = err.get("loc", ()) parts = [str(p) for p in loc if p != "body"] field = ".".join(parts) if parts else "unknown" + + # Strip Pydantic error type prefixes to get clean error message + msg = str(err.get("msg", "")) + prefixes = ["Value error, ", "Type error, ", "Assertion error, "] + for prefix in prefixes: + if msg.startswith(prefix): + msg = msg[len(prefix) :] + break + structured_errors.append( - ValidationErrorDetail( - field=str(field), message=str(err.get("msg", "")) - ) + ValidationErrorDetail(field=str(field), message=msg) ) return cls( From f749d69f3e3709a3e03f3833e53c15b812211d3e Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 23 Mar 2026 09:35:52 +0530 Subject: [PATCH 2/8] credentials: http error and documentation --- backend/app/api/docs/credentials/create.md | 2 +- backend/app/api/docs/onboarding/onboarding.md | 2 +- backend/app/core/providers.py | 2 +- backend/app/crud/credentials.py | 1 + backend/app/models/onboarding.py | 2 +- backend/app/utils.py | 2 +- 6 files changed, 6 insertions(+), 5 deletions(-) diff --git a/backend/app/api/docs/credentials/create.md b/backend/app/api/docs/credentials/create.md index 84c9eeeab..7a4eec80c 100644 --- a/backend/app/api/docs/credentials/create.md +++ b/backend/app/api/docs/credentials/create.md @@ -37,7 +37,7 @@ Credentials are encrypted and stored securely for provider integrations (OpenAI, "api_key": "sk_..." }, "langfuse": { - "public_key": "pk-lf-...", + "public_key": "pk-lf-....", "secret_key": "sk-lf-...", "host": "https://cloud.langfuse.com" } diff --git a/backend/app/api/docs/onboarding/onboarding.md b/backend/app/api/docs/onboarding/onboarding.md index 08c1e6516..a961634a6 100644 --- a/backend/app/api/docs/onboarding/onboarding.md +++ b/backend/app/api/docs/onboarding/onboarding.md @@ -59,7 +59,7 @@ }, { "langfuse": { - "public_key": "pk-lf-...", + "public_key": "pk-lf-....", "secret_key": "sk-lf-...", "host": "https://cloud.langfuse.com" } diff --git a/backend/app/core/providers.py b/backend/app/core/providers.py index 33f687c85..7b924cf22 100644 --- a/backend/app/core/providers.py +++ b/backend/app/core/providers.py @@ -1,5 +1,5 @@ import logging -from typing import Dict, List, Optional +from typing import Dict, List from enum import Enum from dataclasses import dataclass diff --git a/backend/app/crud/credentials.py b/backend/app/crud/credentials.py index d46e94e4d..fdad51b43 100644 --- a/backend/app/crud/credentials.py +++ b/backend/app/crud/credentials.py @@ -11,6 +11,7 @@ from app.core.util import now from app.models import Credential, CredsCreate, CredsUpdate + logger = logging.getLogger(__name__) diff --git a/backend/app/models/onboarding.py b/backend/app/models/onboarding.py index 794f9521d..d40f91f75 100644 --- a/backend/app/models/onboarding.py +++ b/backend/app/models/onboarding.py @@ -114,7 +114,7 @@ def _validate_credential_list(cls, v: list[dict[str, dict[str, str]]] | None): f"Value for provider '{provider_key}' must be an object/dict." ) - # validate_provider_credentials will raise ValueError with clear message if invalid + # validate_provider_credentials will raise ValueError with clear message validate_provider_credentials(provider_key, values) return v diff --git a/backend/app/utils.py b/backend/app/utils.py index 1aa9edf39..d176a6be4 100644 --- a/backend/app/utils.py +++ b/backend/app/utils.py @@ -76,7 +76,7 @@ def failure_response( parts = [str(p) for p in loc if p != "body"] field = ".".join(parts) if parts else "unknown" - # Strip Pydantic error type prefixes to get clean error message + # Strip Pydantic error type prefixes to get better error message msg = str(err.get("msg", "")) prefixes = ["Value error, ", "Type error, ", "Assertion error, "] for prefix in prefixes: From c4d4f0c84448a547b92f26be154fa9c18d17333d Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 23 Mar 2026 11:43:25 +0530 Subject: [PATCH 3/8] test cases and coderabbit review --- backend/app/models/onboarding.py | 8 +++---- .../app/tests/api/routes/test_onboarding.py | 24 +++++++------------ backend/app/tests/core/test_providers.py | 8 ------- backend/app/tests/crud/test_credentials.py | 14 +++++++++-- 4 files changed, 24 insertions(+), 30 deletions(-) diff --git a/backend/app/models/onboarding.py b/backend/app/models/onboarding.py index d40f91f75..48b7197b3 100644 --- a/backend/app/models/onboarding.py +++ b/backend/app/models/onboarding.py @@ -89,13 +89,13 @@ def _validate_credential_list(cls, v: list[dict[str, dict[str, str]]] | None): return v if not isinstance(v, list): - raise TypeError( - "credential must be a list of single-key dicts (e.g., {'openai': {...}})." + raise ValueError( + "Credential must be a list of single-key dicts (e.g., {'openai': {...}})." ) for item in v: if not isinstance(item, dict): - raise TypeError( + raise ValueError( "Credential must be a dict with a single provider key like {'openai': {...}}." ) if len(item) != 1: @@ -110,7 +110,7 @@ def _validate_credential_list(cls, v: list[dict[str, dict[str, str]]] | None): validate_provider(provider_key) if not isinstance(values, dict): - raise TypeError( + raise ValueError( f"Value for provider '{provider_key}' must be an object/dict." ) diff --git a/backend/app/tests/api/routes/test_onboarding.py b/backend/app/tests/api/routes/test_onboarding.py index 3a6f13b76..2d67157dc 100644 --- a/backend/app/tests/api/routes/test_onboarding.py +++ b/backend/app/tests/api/routes/test_onboarding.py @@ -198,9 +198,7 @@ def test_onboard_project_invalid_provider( assert response.status_code == 422 error_response = response.json() assert error_response["errors"] - assert any( - "credential validation failed" in e["message"] for e in error_response["errors"] - ) + assert any("Unsupported provider" in e["message"] for e in error_response["errors"]) def test_onboard_project_non_dict_values_in_credential( @@ -230,9 +228,6 @@ def test_onboard_project_non_dict_values_in_credential( assert response.status_code == 422 error_response = response.json() assert error_response["errors"] - assert any( - "credential validation failed" in e["message"] for e in error_response["errors"] - ) assert any( "must be an object/dict" in e["message"] for e in error_response["errors"] ) @@ -266,9 +261,9 @@ def test_onboard_project_missing_required_fields_for_openai( error_response = response.json() assert error_response["errors"] assert any( - "credential validation failed" in e["message"] for e in error_response["errors"] + "Missing required fields for openai" in e["message"] + for e in error_response["errors"] ) - assert any("openai" in e["message"] for e in error_response["errors"]) def test_onboard_project_missing_required_fields_for_langfuse( @@ -301,15 +296,15 @@ def test_onboard_project_missing_required_fields_for_langfuse( error_response = response.json() assert error_response["errors"] assert any( - "credential validation failed" in e["message"] for e in error_response["errors"] + "Missing required fields for langfuse" in e["message"] + for e in error_response["errors"] ) - assert any("langfuse" in e["message"] for e in error_response["errors"]) def test_onboard_project_aggregates_multiple_credential_errors( client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: - """Test onboarding aggregates multiple credential validation errors with index markers.""" + """Test onboarding reports credential validation errors (fails on first error).""" org_name = "TestOrgOnboard" project_name = "TestProjectOnboard" email = random_email() @@ -336,8 +331,5 @@ def test_onboard_project_aggregates_multiple_credential_errors( assert response.status_code == 422 error_response = response.json() assert error_response["errors"] - assert any( - "credential validation failed" in e["message"] for e in error_response["errors"] - ) - assert any("[0]" in e["message"] for e in error_response["errors"]) - assert any("[1]" in e["message"] for e in error_response["errors"]) + # Validation fails on the first error (unsupported provider) + assert any("Unsupported provider" in e["message"] for e in error_response["errors"]) diff --git a/backend/app/tests/core/test_providers.py b/backend/app/tests/core/test_providers.py index 82c352d1a..1ddc8cf10 100644 --- a/backend/app/tests/core/test_providers.py +++ b/backend/app/tests/core/test_providers.py @@ -21,11 +21,3 @@ def test_validate_provider_credentials_missing_fields(): validate_provider_credentials("openai", {}) assert "Missing required fields" in str(exc_info.value) assert "api_key" in str(exc_info.value) - - # Test AWS missing region - with pytest.raises(ValueError) as exc_info: - validate_provider_credentials( - "aws", {"access_key_id": "test-id", "secret_access_key": "test-secret"} - ) - assert "Missing required fields" in str(exc_info.value) - assert "region" in str(exc_info.value) diff --git a/backend/app/tests/crud/test_credentials.py b/backend/app/tests/crud/test_credentials.py index b85845737..14437f4fb 100644 --- a/backend/app/tests/crud/test_credentials.py +++ b/backend/app/tests/crud/test_credentials.py @@ -206,6 +206,8 @@ def test_remove_creds_for_org(db: Session) -> None: def test_invalid_provider(db: Session) -> None: """Test handling of invalid provider names.""" + from app.core.exception_handlers import HTTPException + project = create_test_project(db) credentials_data = {"invalid_provider": {"api_key": "test-key"}} @@ -214,7 +216,7 @@ def test_invalid_provider(db: Session) -> None: credential=credentials_data, ) - with pytest.raises(ValueError, match="Unsupported provider"): + with pytest.raises(HTTPException) as exc_info: set_creds_for_org( session=db, creds_add=credentials_create, @@ -222,6 +224,9 @@ def test_invalid_provider(db: Session) -> None: project_id=project.id, ) + assert exc_info.value.status_code == 400 + assert "Unsupported provider" in exc_info.value.detail + def test_duplicate_provider_credentials(db: Session) -> None: """Test handling of duplicate provider credentials.""" @@ -253,6 +258,8 @@ def test_duplicate_provider_credentials(db: Session) -> None: def test_langfuse_credential_validation(db: Session) -> None: """Test validation of Langfuse credentials structure.""" + from app.core.exception_handlers import HTTPException + project = create_test_project(db) # Test with missing required fields @@ -268,7 +275,7 @@ def test_langfuse_credential_validation(db: Session) -> None: credential=invalid_credentials, ) - with pytest.raises(ValueError): + with pytest.raises(HTTPException) as exc_info: set_creds_for_org( session=db, creds_add=credentials_create, @@ -276,6 +283,9 @@ def test_langfuse_credential_validation(db: Session) -> None: project_id=project.id, ) + assert exc_info.value.status_code == 400 + assert "Missing required fields for langfuse" in exc_info.value.detail + valid_credentials = { "langfuse": { "public_key": "test-public-key", From 34e7fc21c09cb1426f41fb1bcbdf1c455deb5574 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Tue, 24 Mar 2026 09:26:08 +0530 Subject: [PATCH 4/8] linting issue --- backend/app/celery/tasks/job_execution.py | 30 ++++++--- backend/app/celery/utils.py | 77 ++++++++++++++++------- 2 files changed, 77 insertions(+), 30 deletions(-) diff --git a/backend/app/celery/tasks/job_execution.py b/backend/app/celery/tasks/job_execution.py index aaa763830..e1f9e2442 100644 --- a/backend/app/celery/tasks/job_execution.py +++ b/backend/app/celery/tasks/job_execution.py @@ -1,7 +1,7 @@ import logging +from celery import current_task from asgi_correlation_id import correlation_id -from celery import current_task from app.celery.celery_app import celery_app @@ -70,7 +70,9 @@ def run_doctransform_job(self, project_id: int, job_id: str, trace_id: str, **kw @celery_app.task(bind=True, queue="low_priority", priority=1) -def run_create_collection_job(self, project_id: int, job_id: str, trace_id: str, **kwargs): +def run_create_collection_job( + self, project_id: int, job_id: str, trace_id: str, **kwargs +): from app.services.collections.create_collection import execute_job _set_trace(trace_id) @@ -84,7 +86,9 @@ def run_create_collection_job(self, project_id: int, job_id: str, trace_id: str, @celery_app.task(bind=True, queue="low_priority", priority=1) -def run_delete_collection_job(self, project_id: int, job_id: str, trace_id: str, **kwargs): +def run_delete_collection_job( + self, project_id: int, job_id: str, trace_id: str, **kwargs +): from app.services.collections.delete_collection import execute_job _set_trace(trace_id) @@ -98,7 +102,9 @@ def run_delete_collection_job(self, project_id: int, job_id: str, trace_id: str, @celery_app.task(bind=True, queue="low_priority", priority=1) -def run_stt_batch_submission(self, project_id: int, job_id: str, trace_id: str, **kwargs): +def run_stt_batch_submission( + self, project_id: int, job_id: str, trace_id: str, **kwargs +): from app.services.stt_evaluations.batch_job import execute_batch_submission _set_trace(trace_id) @@ -112,7 +118,9 @@ def run_stt_batch_submission(self, project_id: int, job_id: str, trace_id: str, @celery_app.task(bind=True, queue="low_priority", priority=1) -def run_stt_metric_computation(self, project_id: int, job_id: str, trace_id: str, **kwargs): +def run_stt_metric_computation( + self, project_id: int, job_id: str, trace_id: str, **kwargs +): from app.services.stt_evaluations.metric_job import execute_metric_computation _set_trace(trace_id) @@ -126,7 +134,9 @@ def run_stt_metric_computation(self, project_id: int, job_id: str, trace_id: str @celery_app.task(bind=True, queue="low_priority", priority=1) -def run_tts_batch_submission(self, project_id: int, job_id: str, trace_id: str, **kwargs): +def run_tts_batch_submission( + self, project_id: int, job_id: str, trace_id: str, **kwargs +): from app.services.tts_evaluations.batch_job import execute_batch_submission _set_trace(trace_id) @@ -140,8 +150,12 @@ def run_tts_batch_submission(self, project_id: int, job_id: str, trace_id: str, @celery_app.task(bind=True, queue="low_priority", priority=1) -def run_tts_result_processing(self, project_id: int, job_id: str, trace_id: str, **kwargs): - from app.services.tts_evaluations.batch_result_processing import execute_tts_result_processing +def run_tts_result_processing( + self, project_id: int, job_id: str, trace_id: str, **kwargs +): + from app.services.tts_evaluations.batch_result_processing import ( + execute_tts_result_processing, + ) _set_trace(trace_id) return execute_tts_result_processing( diff --git a/backend/app/celery/utils.py b/backend/app/celery/utils.py index 3fd871724..6e160ea68 100644 --- a/backend/app/celery/utils.py +++ b/backend/app/celery/utils.py @@ -4,7 +4,6 @@ """ import logging from typing import Any, Dict - from celery.result import AsyncResult from app.celery.celery_app import celery_app @@ -12,12 +11,12 @@ logger = logging.getLogger(__name__) -def start_llm_job( - project_id: int, job_id: str, trace_id: str = "N/A", **kwargs -) -> str: +def start_llm_job(project_id: int, job_id: str, trace_id: str = "N/A", **kwargs) -> str: from app.celery.tasks.job_execution import run_llm_job - task = run_llm_job.delay(project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs) + task = run_llm_job.delay( + project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs + ) logger.info(f"[start_llm_job] Started job {job_id} with Celery task {task.id}") return task.id @@ -27,8 +26,12 @@ def start_llm_chain_job( ) -> str: from app.celery.tasks.job_execution import run_llm_chain_job - task = run_llm_chain_job.delay(project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs) - logger.info(f"[start_llm_chain_job] Started job {job_id} with Celery task {task.id}") + task = run_llm_chain_job.delay( + project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs + ) + logger.info( + f"[start_llm_chain_job] Started job {job_id} with Celery task {task.id}" + ) return task.id @@ -37,7 +40,9 @@ def start_response_job( ) -> str: from app.celery.tasks.job_execution import run_response_job - task = run_response_job.delay(project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs) + task = run_response_job.delay( + project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs + ) logger.info(f"[start_response_job] Started job {job_id} with Celery task {task.id}") return task.id @@ -47,8 +52,12 @@ def start_doctransform_job( ) -> str: from app.celery.tasks.job_execution import run_doctransform_job - task = run_doctransform_job.delay(project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs) - logger.info(f"[start_doctransform_job] Started job {job_id} with Celery task {task.id}") + task = run_doctransform_job.delay( + project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs + ) + logger.info( + f"[start_doctransform_job] Started job {job_id} with Celery task {task.id}" + ) return task.id @@ -57,8 +66,12 @@ def start_create_collection_job( ) -> str: from app.celery.tasks.job_execution import run_create_collection_job - task = run_create_collection_job.delay(project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs) - logger.info(f"[start_create_collection_job] Started job {job_id} with Celery task {task.id}") + task = run_create_collection_job.delay( + project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs + ) + logger.info( + f"[start_create_collection_job] Started job {job_id} with Celery task {task.id}" + ) return task.id @@ -67,8 +80,12 @@ def start_delete_collection_job( ) -> str: from app.celery.tasks.job_execution import run_delete_collection_job - task = run_delete_collection_job.delay(project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs) - logger.info(f"[start_delete_collection_job] Started job {job_id} with Celery task {task.id}") + task = run_delete_collection_job.delay( + project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs + ) + logger.info( + f"[start_delete_collection_job] Started job {job_id} with Celery task {task.id}" + ) return task.id @@ -77,8 +94,12 @@ def start_stt_batch_submission( ) -> str: from app.celery.tasks.job_execution import run_stt_batch_submission - task = run_stt_batch_submission.delay(project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs) - logger.info(f"[start_stt_batch_submission] Started job {job_id} with Celery task {task.id}") + task = run_stt_batch_submission.delay( + project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs + ) + logger.info( + f"[start_stt_batch_submission] Started job {job_id} with Celery task {task.id}" + ) return task.id @@ -87,8 +108,12 @@ def start_stt_metric_computation( ) -> str: from app.celery.tasks.job_execution import run_stt_metric_computation - task = run_stt_metric_computation.delay(project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs) - logger.info(f"[start_stt_metric_computation] Started job {job_id} with Celery task {task.id}") + task = run_stt_metric_computation.delay( + project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs + ) + logger.info( + f"[start_stt_metric_computation] Started job {job_id} with Celery task {task.id}" + ) return task.id @@ -97,8 +122,12 @@ def start_tts_batch_submission( ) -> str: from app.celery.tasks.job_execution import run_tts_batch_submission - task = run_tts_batch_submission.delay(project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs) - logger.info(f"[start_tts_batch_submission] Started job {job_id} with Celery task {task.id}") + task = run_tts_batch_submission.delay( + project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs + ) + logger.info( + f"[start_tts_batch_submission] Started job {job_id} with Celery task {task.id}" + ) return task.id @@ -107,8 +136,12 @@ def start_tts_result_processing( ) -> str: from app.celery.tasks.job_execution import run_tts_result_processing - task = run_tts_result_processing.delay(project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs) - logger.info(f"[start_tts_result_processing] Started job {job_id} with Celery task {task.id}") + task = run_tts_result_processing.delay( + project_id=project_id, job_id=job_id, trace_id=trace_id, **kwargs + ) + logger.info( + f"[start_tts_result_processing] Started job {job_id} with Celery task {task.id}" + ) return task.id From 5fcfa8834110bfddeeb58f436a5e4d9f2b4ae9b5 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Tue, 24 Mar 2026 09:30:56 +0530 Subject: [PATCH 5/8] linting issue --- backend/app/celery/celery_app.py | 1 - backend/app/celery/tasks/job_execution.py | 2 +- backend/app/celery/utils.py | 1 + 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/celery/celery_app.py b/backend/app/celery/celery_app.py index 463a1c982..679131503 100644 --- a/backend/app/celery/celery_app.py +++ b/backend/app/celery/celery_app.py @@ -84,4 +84,3 @@ broker_connection_retry_on_startup=True, broker_pool_limit=settings.CELERY_BROKER_POOL_LIMIT, ) - diff --git a/backend/app/celery/tasks/job_execution.py b/backend/app/celery/tasks/job_execution.py index e1f9e2442..a1663179d 100644 --- a/backend/app/celery/tasks/job_execution.py +++ b/backend/app/celery/tasks/job_execution.py @@ -1,7 +1,7 @@ import logging -from celery import current_task from asgi_correlation_id import correlation_id +from celery import current_task from app.celery.celery_app import celery_app diff --git a/backend/app/celery/utils.py b/backend/app/celery/utils.py index 6e160ea68..e4b2a2e3f 100644 --- a/backend/app/celery/utils.py +++ b/backend/app/celery/utils.py @@ -4,6 +4,7 @@ """ import logging from typing import Any, Dict + from celery.result import AsyncResult from app.celery.celery_app import celery_app From df0124d3c636616abbaf6b69e394d45090b03823 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 30 Mar 2026 09:20:44 +0530 Subject: [PATCH 6/8] adding more test cases --- backend/app/api/routes/credentials.py | 12 +- backend/app/tests/api/routes/test_creds.py | 247 +++++++++++++++++++++ 2 files changed, 257 insertions(+), 2 deletions(-) diff --git a/backend/app/api/routes/credentials.py b/backend/app/api/routes/credentials.py index 74e7d9622..8e1e94b41 100644 --- a/backend/app/api/routes/credentials.py +++ b/backend/app/api/routes/credentials.py @@ -85,7 +85,11 @@ def read_provider_credential( provider: str, _current_user: AuthContextDep, ): - provider_enum = validate_provider(provider) + try: + provider_enum = validate_provider(provider) + except ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) + credential = get_provider_credential( session=session, org_id=_current_user.organization_.id, @@ -143,7 +147,11 @@ def delete_provider_credential( provider: str, _current_user: AuthContextDep, ): - provider_enum = validate_provider(provider) + try: + provider_enum = validate_provider(provider) + except ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) + remove_provider_credential( session=session, org_id=_current_user.organization_.id, diff --git a/backend/app/tests/api/routes/test_creds.py b/backend/app/tests/api/routes/test_creds.py index db72a9f4a..9feffa19d 100644 --- a/backend/app/tests/api/routes/test_creds.py +++ b/backend/app/tests/api/routes/test_creds.py @@ -288,3 +288,250 @@ def test_delete_all_when_none_exist_returns_404( ) assert response.status_code == 404 + + +def test_delete_provider_credential( + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test deleting a specific provider's credentials.""" + # First verify the credential exists + get_response = client.get( + f"{settings.API_V1_STR}/credentials/provider/{Provider.OPENAI.value}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert get_response.status_code == 200 + + # Delete the credential + response = client.delete( + f"{settings.API_V1_STR}/credentials/provider/{Provider.OPENAI.value}", + headers={"X-API-KEY": user_api_key.key}, + ) + + assert response.status_code == 200 + response_data = response.json() + assert ( + response_data["data"]["message"] == "Provider credentials removed successfully" + ) + + # Verify it's deleted + verify_response = client.get( + f"{settings.API_V1_STR}/credentials/provider/{Provider.OPENAI.value}", + headers={"X-API-KEY": user_api_key.key}, + ) + assert verify_response.status_code == 404 + + +def test_delete_provider_credential_not_found( + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test deleting a provider credential that doesn't exist.""" + # Delete if exists + client.delete( + f"{settings.API_V1_STR}/credentials/provider/{Provider.OPENAI.value}", + headers={"X-API-KEY": user_api_key.key}, + ) + + # Try to delete again + response = client.delete( + f"{settings.API_V1_STR}/credentials/provider/{Provider.OPENAI.value}", + headers={"X-API-KEY": user_api_key.key}, + ) + + assert response.status_code == 404 + + +def test_delete_provider_credential_invalid_provider( + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test deleting credentials with invalid provider name.""" + response = client.delete( + f"{settings.API_V1_STR}/credentials/provider/invalid_provider", + headers={"X-API-KEY": user_api_key.key}, + ) + + # ValueError from validate_provider is caught and converted to HTTPException(400) + assert response.status_code == 400 + assert "Unsupported provider" in response.json()["error"] + + +def test_read_provider_credential_invalid_provider( + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test reading credentials with invalid provider name.""" + response = client.get( + f"{settings.API_V1_STR}/credentials/provider/invalid_provider", + headers={"X-API-KEY": user_api_key.key}, + ) + + # ValueError from validate_provider is caught and converted to HTTPException(400) + assert response.status_code == 400 + assert "Unsupported provider" in response.json()["error"] + + +def test_create_credential_missing_credential_field( + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test creating credentials without the credential field.""" + credential_data = { + "organization_id": user_api_key.organization_id, + "project_id": user_api_key.project_id, + "is_active": True, + } + + response = client.post( + f"{settings.API_V1_STR}/credentials/", + json=credential_data, + headers={"X-API-KEY": user_api_key.key}, + ) + + assert response.status_code == 400 # Validation error + + +def test_create_credential_empty_credential_dict( + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test creating credentials with empty credential dictionary.""" + credential_data = { + "organization_id": user_api_key.organization_id, + "project_id": user_api_key.project_id, + "is_active": True, + "credential": {}, + } + + response = client.post( + f"{settings.API_V1_STR}/credentials/", + json=credential_data, + headers={"X-API-KEY": user_api_key.key}, + ) + + assert response.status_code == 400 # Should fail to create credentials + + +def test_update_credential_missing_provider_field( + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test updating credentials without provider field.""" + update_data = { + "credential": { + "api_key": "sk-test123", + "model": "gpt-4", + }, + } + + response = client.patch( + f"{settings.API_V1_STR}/credentials/", + json=update_data, + headers={"X-API-KEY": user_api_key.key}, + ) + + # FastAPI Pydantic validation catches missing required field before route handler runs + assert response.status_code == 422 + + +def test_update_credential_missing_credential_field( + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test updating credentials without credential field.""" + update_data = { + "provider": Provider.OPENAI.value, + } + + response = client.patch( + f"{settings.API_V1_STR}/credentials/", + json=update_data, + headers={"X-API-KEY": user_api_key.key}, + ) + + # FastAPI Pydantic validation catches missing required field before route handler runs + assert response.status_code == 422 + + +def test_update_credential_empty_credential( + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test updating credentials with empty credential object.""" + update_data = { + "provider": Provider.OPENAI.value, + "credential": {}, + } + + response = client.patch( + f"{settings.API_V1_STR}/credentials/", + json=update_data, + headers={"X-API-KEY": user_api_key.key}, + ) + + # Should still update but with empty credential + assert response.status_code in [200, 400] # Depends on implementation + + +def test_read_credentials_not_found( + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test reading credentials when none exist.""" + # Delete all credentials first + client.delete( + f"{settings.API_V1_STR}/credentials/", + headers={"X-API-KEY": user_api_key.key}, + ) + + response = client.get( + f"{settings.API_V1_STR}/credentials/", + headers={"X-API-KEY": user_api_key.key}, + ) + + assert response.status_code == 404 + assert "Credentials not found" in response.json()["error"] + + +def test_create_multiple_providers_at_once( + client: TestClient, + user_api_key: TestAuthContext, +) -> None: + """Test creating credentials for multiple providers in a single request.""" + # Delete all credentials first + client.delete( + f"{settings.API_V1_STR}/credentials/", + headers={"X-API-KEY": user_api_key.key}, + ) + + credential_data = { + "organization_id": user_api_key.organization_id, + "project_id": user_api_key.project_id, + "is_active": True, + "credential": { + Provider.OPENAI.value: { + "api_key": "sk-" + generate_random_string(), + "model": "gpt-4", + "temperature": 0.7, + }, + Provider.LANGFUSE.value: { + "secret_key": "sk-lf-" + generate_random_string(), + "public_key": "pk-lf-" + generate_random_string(), + "host": "https://cloud.langfuse.com", + }, + }, + } + + response = client.post( + f"{settings.API_V1_STR}/credentials/", + json=credential_data, + headers={"X-API-KEY": user_api_key.key}, + ) + + assert response.status_code == 200 + data = response.json()["data"] + assert len(data) == 2 + providers = [cred["provider"] for cred in data] + assert Provider.OPENAI.value in providers + assert Provider.LANGFUSE.value in providers From 294f4881a4a99508c0e82c20c3c9a9fd91e57489 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Tue, 31 Mar 2026 09:30:43 +0530 Subject: [PATCH 7/8] pr review fixes --- backend/app/api/docs/credentials/create.md | 2 +- backend/app/core/providers.py | 6 - backend/app/crud/credentials.py | 3 - .../app/tests/api/routes/test_onboarding.py | 387 ++++++++++++++++++ 4 files changed, 388 insertions(+), 10 deletions(-) diff --git a/backend/app/api/docs/credentials/create.md b/backend/app/api/docs/credentials/create.md index 7a4eec80c..df5779201 100644 --- a/backend/app/api/docs/credentials/create.md +++ b/backend/app/api/docs/credentials/create.md @@ -1,6 +1,6 @@ Persist new credentials for the current organization and project. -Credentials are encrypted and stored securely for provider integrations (OpenAI, Langfuse, etc.). Only one credential per provider is allowed per organization-project combination. +Credentials are encrypted and stored securely for provider integrations (OpenAI, Langfuse, etc.). Only one credential per provider is allowed per organization-project combination. You can send credentials for a single provider or multiple providers in one request. Refer to the examples below for the required input parameters for each provider. ### Supported Providers: - **LLM:** openai, sarvamai, google(gemini) diff --git a/backend/app/core/providers.py b/backend/app/core/providers.py index 7b924cf22..7248ea1df 100644 --- a/backend/app/core/providers.py +++ b/backend/app/core/providers.py @@ -16,9 +16,6 @@ class Provider(str, Enum): ELEVENLABS = "elevenlabs" -# AWS = "aws" - - @dataclass class ProviderConfig: """Configuration for a provider including its required credential fields.""" @@ -35,9 +32,6 @@ class ProviderConfig: Provider.GOOGLE: ProviderConfig(required_fields=["api_key"]), Provider.SARVAMAI: ProviderConfig(required_fields=["api_key"]), Provider.ELEVENLABS: ProviderConfig(required_fields=["api_key"]), - # Provider.AWS: ProviderConfig( - # required_fields=["access_key_id", "secret_access_key", "region"] - # ), } diff --git a/backend/app/crud/credentials.py b/backend/app/crud/credentials.py index fdad51b43..6471d52d9 100644 --- a/backend/app/crud/credentials.py +++ b/backend/app/crud/credentials.py @@ -28,9 +28,7 @@ def set_creds_for_org( raise HTTPException(400, "No credentials provided") for provider, credentials in creds_add.credential.items(): - # Validate provider and credentials try: - validate_provider(provider) validate_provider_credentials(provider, credentials) except ValueError as e: logger.error( @@ -190,7 +188,6 @@ def update_creds_for_org( raise ValueError("Provider and credential must be provided") try: - validate_provider(creds_in.provider) validate_provider_credentials(creds_in.provider, creds_in.credential) except ValueError as e: logger.error( diff --git a/backend/app/tests/api/routes/test_onboarding.py b/backend/app/tests/api/routes/test_onboarding.py index 2d67157dc..d32c195ac 100644 --- a/backend/app/tests/api/routes/test_onboarding.py +++ b/backend/app/tests/api/routes/test_onboarding.py @@ -333,3 +333,390 @@ def test_onboard_project_aggregates_multiple_credential_errors( assert error_response["errors"] # Validation fails on the first error (unsupported provider) assert any("Unsupported provider" in e["message"] for e in error_response["errors"]) + + +def test_onboard_project_credentials_not_a_list( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding fails when credentials is not a list.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": {"openai": {"api_key": "sk-test"}}, # Should be a list + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 422 + error_response = response.json() + assert error_response["errors"] + # Pydantic catches this before custom validator - returns type error + assert any( + "Input should be a valid list" in e["message"] for e in error_response["errors"] + ) + + +def test_onboard_project_credentials_string_instead_of_list( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding fails when credentials is a string instead of list.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": "sk-test-key", # Should be a list + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 422 + error_response = response.json() + assert error_response["errors"] + # Pydantic catches this before custom validator - returns type error + assert any( + "Input should be a valid list" in e["message"] for e in error_response["errors"] + ) + + +def test_onboard_project_credential_item_not_a_dict( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding fails when a credential item is not a dict.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": ["sk-test-key"], # Items should be dicts + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 422 + error_response = response.json() + assert error_response["errors"] + # Pydantic catches this before custom validator - returns type error + assert any( + "Input should be a valid dictionary" in e["message"] + for e in error_response["errors"] + ) + + +def test_onboard_project_credential_item_with_multiple_provider_keys( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding fails when credential item has multiple provider keys.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": [ + { + "openai": {"api_key": "sk-test"}, + "langfuse": { + "secret_key": "sk-lf", + "public_key": "pk-lf", + "host": "https://cloud.langfuse.com", + }, + } + ], # Should have exactly one provider key per dict + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 422 + error_response = response.json() + assert error_response["errors"] + assert any( + "must have exactly one provider key" in e["message"] + for e in error_response["errors"] + ) + + +def test_onboard_project_credential_item_empty_dict( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding fails when credential item is an empty dict.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": [{}], # Empty dict - no provider key + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 422 + error_response = response.json() + assert error_response["errors"] + assert any( + "must have exactly one provider key" in e["message"] + for e in error_response["errors"] + ) + + +def test_onboard_project_credentials_empty_list( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding succeeds with empty credentials list (treated same as None).""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": [], # Empty list is valid + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 201 + response_data = response.json() + assert response_data["success"] is True + # Should not have metadata about credentials + assert response_data.get("metadata") is None + + +def test_onboard_project_with_google_credentials( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding with Google credentials.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + google_api_key = f"AIza{random_lower_string()}" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": [{"google": {"api_key": google_api_key}}], + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 201 + response_data = response.json() + assert response_data["success"] is True + assert ( + response_data["metadata"]["note"] + == "Given credential(s) have been saved for this project." + ) + + +def test_onboard_project_with_sarvamai_credentials( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding with SarvamAI credentials.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + sarvamai_api_key = f"sarvam-{random_lower_string()}" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": [{"sarvamai": {"api_key": sarvamai_api_key}}], + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 201 + response_data = response.json() + assert response_data["success"] is True + assert ( + response_data["metadata"]["note"] + == "Given credential(s) have been saved for this project." + ) + + +def test_onboard_project_with_elevenlabs_credentials( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding with ElevenLabs credentials.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + elevenlabs_api_key = f"el-{random_lower_string()}" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": [{"elevenlabs": {"api_key": elevenlabs_api_key}}], + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 201 + response_data = response.json() + assert response_data["success"] is True + assert ( + response_data["metadata"]["note"] + == "Given credential(s) have been saved for this project." + ) + + +def test_onboard_project_with_all_supported_providers( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding with credentials for all supported providers.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": [ + {"openai": {"api_key": f"sk-{random_lower_string()}"}}, + { + "langfuse": { + "secret_key": f"sk-lf-{random_lower_string()}", + "public_key": f"pk-lf-{random_lower_string()}", + "host": "https://cloud.langfuse.com", + } + }, + {"google": {"api_key": f"AIza{random_lower_string()}"}}, + {"sarvamai": {"api_key": f"sarvam-{random_lower_string()}"}}, + {"elevenlabs": {"api_key": f"el-{random_lower_string()}"}}, + ], + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 201 + response_data = response.json() + assert response_data["success"] is True + assert ( + response_data["metadata"]["note"] + == "Given credential(s) have been saved for this project." + ) + + +def test_onboard_project_google_missing_api_key( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding fails when Google credential is missing api_key.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": [{"google": {}}], # missing api_key + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 422 + error_response = response.json() + assert error_response["errors"] + assert any( + "Missing required fields for google" in e["message"] + for e in error_response["errors"] + ) + + +def test_onboard_project_sarvamai_missing_api_key( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding fails when SarvamAI credential is missing api_key.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": [{"sarvamai": {}}], # missing api_key + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 422 + error_response = response.json() + assert error_response["errors"] + assert any( + "Missing required fields for sarvamai" in e["message"] + for e in error_response["errors"] + ) + + +def test_onboard_project_elevenlabs_missing_api_key( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + """Test onboarding fails when ElevenLabs credential is missing api_key.""" + org_name = "TestOrgOnboard" + project_name = "TestProjectOnboard" + + onboard_data = { + "organization_name": org_name, + "project_name": project_name, + "credentials": [{"elevenlabs": {}}], # missing api_key + } + + response = client.post( + f"{settings.API_V1_STR}/onboard", + json=onboard_data, + headers=superuser_token_headers, + ) + + assert response.status_code == 422 + error_response = response.json() + assert error_response["errors"] + assert any( + "Missing required fields for elevenlabs" in e["message"] + for e in error_response["errors"] + ) From 077da6acbe47d82b377e64705fc6a070870407cd Mon Sep 17 00:00:00 2001 From: nishika26 Date: Thu, 2 Apr 2026 01:23:10 +0530 Subject: [PATCH 8/8] pr review fixes --- backend/app/crud/credentials.py | 11 ----------- backend/app/models/onboarding.py | 6 ++---- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/backend/app/crud/credentials.py b/backend/app/crud/credentials.py index 6471d52d9..e6c1ded6e 100644 --- a/backend/app/crud/credentials.py +++ b/backend/app/crud/credentials.py @@ -152,9 +152,6 @@ def get_provider_credential( try: validate_provider(provider) except ValueError as e: - logger.error( - f"[get_provider_credential] Validation error | organization_id: {org_id}, project_id: {project_id}, provider: {provider}, error: {str(e)}" - ) raise HTTPException(status_code=400, detail=str(e)) statement = select(Credential).where( @@ -232,14 +229,6 @@ def remove_provider_credential( Raises: HTTPException: If credentials not found or deletion fails """ - try: - validate_provider(provider) - except ValueError as e: - logger.error( - f"[remove_provider_credential] Validation error | organization_id: {org_id}, project_id: {project_id}, provider: {provider}, error: {str(e)}" - ) - raise HTTPException(status_code=400, detail=str(e)) - # Verify credentials exist before attempting delete creds = get_provider_credential( session=session, diff --git a/backend/app/models/onboarding.py b/backend/app/models/onboarding.py index 48b7197b3..942d36134 100644 --- a/backend/app/models/onboarding.py +++ b/backend/app/models/onboarding.py @@ -89,13 +89,13 @@ def _validate_credential_list(cls, v: list[dict[str, dict[str, str]]] | None): return v if not isinstance(v, list): - raise ValueError( + raise TypeError( "Credential must be a list of single-key dicts (e.g., {'openai': {...}})." ) for item in v: if not isinstance(item, dict): - raise ValueError( + raise TypeError( "Credential must be a dict with a single provider key like {'openai': {...}}." ) if len(item) != 1: @@ -106,7 +106,6 @@ def _validate_credential_list(cls, v: list[dict[str, dict[str, str]]] | None): (provider_key,) = item.keys() values = item[provider_key] - # validate_provider will raise ValueError with clear message if invalid validate_provider(provider_key) if not isinstance(values, dict): @@ -114,7 +113,6 @@ def _validate_credential_list(cls, v: list[dict[str, dict[str, str]]] | None): f"Value for provider '{provider_key}' must be an object/dict." ) - # validate_provider_credentials will raise ValueError with clear message validate_provider_credentials(provider_key, values) return v