From 58bdf9d684ade052f219b4733b936924250cee9d Mon Sep 17 00:00:00 2001 From: pablodanswer Date: Wed, 11 Sep 2024 22:38:15 -0700 Subject: [PATCH] Add connector deletion failure message (#2392) --- ...877_add_ccpair_deletion_failure_message.py | 27 +++++++++++++++++++ .../danswer/background/celery/celery_app.py | 24 +++++++++++------ .../danswer/db/connector_credential_pair.py | 12 +++++++++ backend/danswer/db/models.py | 3 +++ backend/danswer/server/documents/models.py | 2 ++ .../[ccPairId]/DeletionErrorStatus.tsx | 25 +++++++++++++++++ .../app/admin/connector/[ccPairId]/page.tsx | 12 +++++++++ .../app/admin/connector/[ccPairId]/types.ts | 1 + web/tailwind-themes/tailwind.config.js | 12 +++++++++ 9 files changed, 110 insertions(+), 8 deletions(-) create mode 100644 backend/alembic/versions/0ebb1d516877_add_ccpair_deletion_failure_message.py create mode 100644 web/src/app/admin/connector/[ccPairId]/DeletionErrorStatus.tsx diff --git a/backend/alembic/versions/0ebb1d516877_add_ccpair_deletion_failure_message.py b/backend/alembic/versions/0ebb1d516877_add_ccpair_deletion_failure_message.py new file mode 100644 index 00000000000..526c9449fce --- /dev/null +++ b/backend/alembic/versions/0ebb1d516877_add_ccpair_deletion_failure_message.py @@ -0,0 +1,27 @@ +"""add ccpair deletion failure message + +Revision ID: 0ebb1d516877 +Revises: 52a219fb5233 +Create Date: 2024-09-10 15:03:48.233926 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "0ebb1d516877" +down_revision = "52a219fb5233" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column( + "connector_credential_pair", + sa.Column("deletion_failure_message", sa.String(), nullable=True), + ) + + +def downgrade() -> None: + op.drop_column("connector_credential_pair", "deletion_failure_message") diff --git a/backend/danswer/background/celery/celery_app.py b/backend/danswer/background/celery/celery_app.py index 5029520dcb6..a48d8aa4a15 100644 --- a/backend/danswer/background/celery/celery_app.py +++ b/backend/danswer/background/celery/celery_app.py @@ -1,4 +1,5 @@ import json +import traceback from datetime import timedelta from typing import Any from typing import cast @@ -40,6 +41,7 @@ from danswer.configs.constants import PostgresAdvisoryLocks from danswer.connectors.factory import instantiate_connector from danswer.connectors.models import InputType +from danswer.db.connector_credential_pair import add_deletion_failure_message from danswer.db.connector_credential_pair import ( get_connector_credential_pair, ) @@ -97,6 +99,7 @@ def cleanup_connector_credential_pair_task( Needs to potentially update a large number of Postgres and Vespa docs, including deleting them or updating the ACL""" engine = get_sqlalchemy_engine() + with Session(engine) as db_session: # validate that the connector / credential pair is deletable cc_pair = get_connector_credential_pair( @@ -109,14 +112,13 @@ def cleanup_connector_credential_pair_task( f"Cannot run deletion attempt - connector_credential_pair with Connector ID: " f"{connector_id} and Credential ID: {credential_id} does not exist." ) - - deletion_attempt_disallowed_reason = check_deletion_attempt_is_allowed( - connector_credential_pair=cc_pair, db_session=db_session - ) - if deletion_attempt_disallowed_reason: - raise ValueError(deletion_attempt_disallowed_reason) - try: + deletion_attempt_disallowed_reason = check_deletion_attempt_is_allowed( + connector_credential_pair=cc_pair, db_session=db_session + ) + if deletion_attempt_disallowed_reason: + raise ValueError(deletion_attempt_disallowed_reason) + # The bulk of the work is in here, updates Postgres and Vespa curr_ind_name, sec_ind_name = get_both_index_names(db_session) document_index = get_default_document_index( @@ -127,10 +129,15 @@ def cleanup_connector_credential_pair_task( document_index=document_index, cc_pair=cc_pair, ) + except Exception as e: + stack_trace = traceback.format_exc() + error_message = f"Error: {str(e)}\n\nStack Trace:\n{stack_trace}" + add_deletion_failure_message(db_session, cc_pair.id, error_message) task_logger.exception( f"Failed to run connector_deletion. " - f"connector_id={connector_id} credential_id={credential_id}" + f"connector_id={connector_id} credential_id={credential_id}\n" + f"Stack Trace:\n{stack_trace}" ) raise e @@ -410,6 +417,7 @@ def check_for_cc_pair_deletion_task() -> None: task_logger.info( f"Deleting the {cc_pair.name} connector credential pair" ) + cleanup_connector_credential_pair_task.apply_async( kwargs=dict( connector_id=cc_pair.connector.id, diff --git a/backend/danswer/db/connector_credential_pair.py b/backend/danswer/db/connector_credential_pair.py index f35aed9186c..004b5a754e4 100644 --- a/backend/danswer/db/connector_credential_pair.py +++ b/backend/danswer/db/connector_credential_pair.py @@ -98,6 +98,18 @@ def get_connector_credential_pairs( return list(results.all()) +def add_deletion_failure_message( + db_session: Session, + cc_pair_id: int, + failure_message: str, +) -> None: + cc_pair = get_connector_credential_pair_from_id(cc_pair_id, db_session) + if not cc_pair: + return + cc_pair.deletion_failure_message = failure_message + db_session.commit() + + def get_cc_pair_groups_for_ids( db_session: Session, cc_pair_ids: list[int], diff --git a/backend/danswer/db/models.py b/backend/danswer/db/models.py index adceeea17b8..c0d24770704 100644 --- a/backend/danswer/db/models.py +++ b/backend/danswer/db/models.py @@ -375,6 +375,9 @@ class ConnectorCredentialPair(Base): connector_id: Mapped[int] = mapped_column( ForeignKey("connector.id"), primary_key=True ) + + deletion_failure_message: Mapped[str | None] = mapped_column(String, nullable=True) + credential_id: Mapped[int] = mapped_column( ForeignKey("credential.id"), primary_key=True ) diff --git a/backend/danswer/server/documents/models.py b/backend/danswer/server/documents/models.py index 61e386638d5..517813892b8 100644 --- a/backend/danswer/server/documents/models.py +++ b/backend/danswer/server/documents/models.py @@ -220,6 +220,7 @@ class CCPairFullInfo(BaseModel): latest_deletion_attempt: DeletionAttemptSnapshot | None is_public: bool is_editable_for_current_user: bool + deletion_failure_message: str | None @classmethod def from_models( @@ -262,6 +263,7 @@ def from_models( latest_deletion_attempt=latest_deletion_attempt, is_public=cc_pair_model.is_public, is_editable_for_current_user=is_editable_for_current_user, + deletion_failure_message=cc_pair_model.deletion_failure_message, ) diff --git a/web/src/app/admin/connector/[ccPairId]/DeletionErrorStatus.tsx b/web/src/app/admin/connector/[ccPairId]/DeletionErrorStatus.tsx new file mode 100644 index 00000000000..dbeb28cf631 --- /dev/null +++ b/web/src/app/admin/connector/[ccPairId]/DeletionErrorStatus.tsx @@ -0,0 +1,25 @@ +import { FiInfo } from "react-icons/fi"; + +export default function DeletionErrorStatus({ + deletion_failure_message, +}: { + deletion_failure_message: string; +}) { + return ( +
+
+

Deletion Error

+
+ +
+ This error occurred while attempting to delete the connector. You + may re-attempt a deletion by clicking the "Delete" button. +
+
+
+
+

{deletion_failure_message}

+
+
+ ); +} diff --git a/web/src/app/admin/connector/[ccPairId]/page.tsx b/web/src/app/admin/connector/[ccPairId]/page.tsx index f2e8a8de8cc..b18ab24f103 100644 --- a/web/src/app/admin/connector/[ccPairId]/page.tsx +++ b/web/src/app/admin/connector/[ccPairId]/page.tsx @@ -22,6 +22,7 @@ import { useEffect, useRef, useState } from "react"; import { CheckmarkIcon, EditIcon, XIcon } from "@/components/icons/icons"; import { usePopup } from "@/components/admin/connectors/Popup"; import { updateConnectorCredentialPairName } from "@/lib/connector"; +import DeletionErrorStatus from "./DeletionErrorStatus"; // since the uploaded files are cleaned up after some period of time // re-indexing will not work for the file connector. Also, it would not @@ -184,6 +185,17 @@ function Main({ ccPairId }: { ccPairId: number }) { : "This connector belongs to groups where you don't have curator permissions, so it's not editable."} )} + + {ccPair.deletion_failure_message && + ccPair.status === ConnectorCredentialPairStatus.DELETING && ( + <> +
+ + + )} + {credentialTemplates[ccPair.connector.source] && ccPair.is_editable_for_current_user && ( <> diff --git a/web/src/app/admin/connector/[ccPairId]/types.ts b/web/src/app/admin/connector/[ccPairId]/types.ts index f44b958b095..55bbe955730 100644 --- a/web/src/app/admin/connector/[ccPairId]/types.ts +++ b/web/src/app/admin/connector/[ccPairId]/types.ts @@ -24,6 +24,7 @@ export interface CCPairFullInfo { latest_deletion_attempt: DeletionAttemptSnapshot | null; is_public: boolean; is_editable_for_current_user: boolean; + deletion_failure_message: string | null; } export interface PaginatedIndexAttempts { diff --git a/web/tailwind-themes/tailwind.config.js b/web/tailwind-themes/tailwind.config.js index 1cac0c877a1..f9d3fb93bd9 100644 --- a/web/tailwind-themes/tailwind.config.js +++ b/web/tailwind-themes/tailwind.config.js @@ -88,6 +88,18 @@ module.exports = { input: "#ffffff", + // Error colors + "error-50": "#fef2f2", + "error-100": "#fee2e2", + "error-200": "#fecaca", + "error-300": "#fca5a5", + "error-400": "#f87171", + "error-500": "#ef4444", + "error-600": "#dc2626", + "error-700": "#b91c1c", + "error-800": "#991b1b", + "error-900": "#7f1d1d", + background: "#fafafa", // 50 "background-100": "#f5f5f5", // neutral-100 "background-125": "#F1F2F4", // gray-125