From e255ff7d23e1ba65df7d47c26f8a018e799cfa27 Mon Sep 17 00:00:00 2001 From: rkuo-danswer Date: Wed, 11 Dec 2024 11:04:09 -0800 Subject: [PATCH] editable refresh and prune for connectors (#3406) * editable refresh and prune for connectors * add extra validations on pruning/refresh frequency * fix validation * fix icon usage * fix TextFormField error formatting * nit --------- Co-authored-by: Richard Kuo (Danswer) Co-authored-by: pablodanswer --- backend/danswer/db/models.py | 19 +++ backend/danswer/server/documents/cc_pair.py | 41 ++++++ backend/danswer/server/documents/models.py | 5 + .../connector/[ccPairId]/ConfigDisplay.tsx | 23 ++- .../app/admin/connector/[ccPairId]/page.tsx | 136 +++++++++++++++++- web/src/components/admin/connectors/Field.tsx | 2 +- .../components/modals/EditPropertyModal.tsx | 71 +++++++++ web/src/lib/connector.ts | 17 +++ 8 files changed, 310 insertions(+), 4 deletions(-) create mode 100644 web/src/components/modals/EditPropertyModal.tsx diff --git a/backend/danswer/db/models.py b/backend/danswer/db/models.py index 7e564829b..031500360 100644 --- a/backend/danswer/db/models.py +++ b/backend/danswer/db/models.py @@ -568,6 +568,25 @@ class Connector(Base): list["DocumentByConnectorCredentialPair"] ] = relationship("DocumentByConnectorCredentialPair", back_populates="connector") + # synchronize this validation logic with RefreshFrequencySchema etc on front end + # until we have a centralized validation schema + + # TODO(rkuo): experiment with SQLAlchemy validators rather than manual checks + # https://docs.sqlalchemy.org/en/20/orm/mapped_attributes.html + def validate_refresh_freq(self) -> None: + if self.refresh_freq is not None: + if self.refresh_freq < 60: + raise ValueError( + "refresh_freq must be greater than or equal to 60 seconds." + ) + + def validate_prune_freq(self) -> None: + if self.prune_freq is not None: + if self.prune_freq < 86400: + raise ValueError( + "prune_freq must be greater than or equal to 86400 seconds." + ) + class Credential(Base): __tablename__ = "credential" diff --git a/backend/danswer/server/documents/cc_pair.py b/backend/danswer/server/documents/cc_pair.py index 46bdb2078..424ae2564 100644 --- a/backend/danswer/server/documents/cc_pair.py +++ b/backend/danswer/server/documents/cc_pair.py @@ -45,6 +45,7 @@ from danswer.db.search_settings import get_current_search_settings from danswer.redis.redis_connector import RedisConnector from danswer.redis.redis_pool import get_redis_client from danswer.server.documents.models import CCPairFullInfo +from danswer.server.documents.models import CCPropertyUpdateRequest from danswer.server.documents.models import CCStatusUpdateRequest from danswer.server.documents.models import ConnectorCredentialPairIdentifier from danswer.server.documents.models import ConnectorCredentialPairMetadata @@ -308,6 +309,46 @@ def update_cc_pair_name( raise HTTPException(status_code=400, detail="Name must be unique") +@router.put("/admin/cc-pair/{cc_pair_id}/property") +def update_cc_pair_property( + cc_pair_id: int, + update_request: CCPropertyUpdateRequest, # in seconds + user: User | None = Depends(current_curator_or_admin_user), + db_session: Session = Depends(get_session), +) -> StatusResponse[int]: + cc_pair = get_connector_credential_pair_from_id( + cc_pair_id=cc_pair_id, + db_session=db_session, + user=user, + get_editable=True, + ) + if not cc_pair: + raise HTTPException( + status_code=400, detail="CC Pair not found for current user's permissions" + ) + + # Can we centralize logic for updating connector properties + # so that we don't need to manually validate everywhere? + if update_request.name == "refresh_frequency": + cc_pair.connector.refresh_freq = int(update_request.value) + cc_pair.connector.validate_refresh_freq() + db_session.commit() + + msg = "Refresh frequency updated successfully" + elif update_request.name == "pruning_frequency": + cc_pair.connector.prune_freq = int(update_request.value) + cc_pair.connector.validate_prune_freq() + db_session.commit() + + msg = "Pruning frequency updated successfully" + else: + raise HTTPException( + status_code=400, detail=f"Property name {update_request.name} is not valid." + ) + + return StatusResponse(success=True, message=msg, data=cc_pair_id) + + @router.get("/admin/cc-pair/{cc_pair_id}/last_pruned") def get_cc_pair_last_pruned( cc_pair_id: int, diff --git a/backend/danswer/server/documents/models.py b/backend/danswer/server/documents/models.py index 7b523d929..2c4f50944 100644 --- a/backend/danswer/server/documents/models.py +++ b/backend/danswer/server/documents/models.py @@ -364,6 +364,11 @@ class RunConnectorRequest(BaseModel): from_beginning: bool = False +class CCPropertyUpdateRequest(BaseModel): + name: str + value: str + + """Connectors Models""" diff --git a/web/src/app/admin/connector/[ccPairId]/ConfigDisplay.tsx b/web/src/app/admin/connector/[ccPairId]/ConfigDisplay.tsx index e34f2ccb1..74a4391e4 100644 --- a/web/src/app/admin/connector/[ccPairId]/ConfigDisplay.tsx +++ b/web/src/app/admin/connector/[ccPairId]/ConfigDisplay.tsx @@ -2,6 +2,7 @@ import CardSection from "@/components/admin/CardSection"; import { getNameFromPath } from "@/lib/fileUtils"; import { ValidSources } from "@/lib/types"; import Title from "@/components/ui/title"; +import { EditIcon } from "@/components/icons/icons"; import { useState } from "react"; import { ChevronUpIcon } from "lucide-react"; @@ -112,10 +113,14 @@ export function AdvancedConfigDisplay({ pruneFreq, refreshFreq, indexingStart, + onRefreshEdit, + onPruningEdit, }: { pruneFreq: number | null; refreshFreq: number | null; indexingStart: Date | null; + onRefreshEdit: () => void; + onPruningEdit: () => void; }) { const formatRefreshFrequency = (seconds: number | null): string => { if (seconds === null) return "-"; @@ -151,7 +156,14 @@ export function AdvancedConfigDisplay({ className="w-full flex justify-between items-center py-2" > Pruning Frequency - {formatPruneFrequency(pruneFreq)} + + {formatPruneFrequency(pruneFreq)} + + + + )} {refreshFreq && ( @@ -160,7 +172,14 @@ export function AdvancedConfigDisplay({ className="w-full flex justify-between items-center py-2" > Refresh Frequency - {formatRefreshFrequency(refreshFreq)} + + {formatRefreshFrequency(refreshFreq)} + + + + )} {indexingStart && ( diff --git a/web/src/app/admin/connector/[ccPairId]/page.tsx b/web/src/app/admin/connector/[ccPairId]/page.tsx index c23775593..7bc422987 100644 --- a/web/src/app/admin/connector/[ccPairId]/page.tsx +++ b/web/src/app/admin/connector/[ccPairId]/page.tsx @@ -7,7 +7,10 @@ import { SourceIcon } from "@/components/SourceIcon"; import { CCPairStatus } from "@/components/Status"; import { usePopup } from "@/components/admin/connectors/Popup"; import CredentialSection from "@/components/credentials/CredentialSection"; -import { updateConnectorCredentialPairName } from "@/lib/connector"; +import { + updateConnectorCredentialPairName, + updateConnectorCredentialPairProperty, +} from "@/lib/connector"; import { credentialTemplates } from "@/lib/connectors/credentials"; import { errorHandlingFetcher } from "@/lib/fetcher"; import { ValidSources } from "@/lib/types"; @@ -26,12 +29,33 @@ import { buildCCPairInfoUrl } from "./lib"; import { CCPairFullInfo, ConnectorCredentialPairStatus } from "./types"; import { EditableStringFieldDisplay } from "@/components/EditableStringFieldDisplay"; import { Button } from "@/components/ui/button"; +import EditPropertyModal from "@/components/modals/EditPropertyModal"; + +import * as Yup from "yup"; // 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 // make sense to re-index, since the files will not have changed. const CONNECTOR_TYPES_THAT_CANT_REINDEX: ValidSources[] = [ValidSources.File]; +// synchronize these validations with the SQLAlchemy connector class until we have a +// centralized schema for both frontend and backend +const RefreshFrequencySchema = Yup.object().shape({ + propertyValue: Yup.number() + .typeError("Property value must be a valid number") + .integer("Property value must be an integer") + .min(60, "Property value must be greater than or equal to 60") + .required("Property value is required"), +}); + +const PruneFrequencySchema = Yup.object().shape({ + propertyValue: Yup.number() + .typeError("Property value must be a valid number") + .integer("Property value must be an integer") + .min(86400, "Property value must be greater than or equal to 86400") + .required("Property value is required"), +}); + function Main({ ccPairId }: { ccPairId: number }) { const router = useRouter(); // Initialize the router const { @@ -45,6 +69,8 @@ function Main({ ccPairId }: { ccPairId: number }) { ); const [hasLoadedOnce, setHasLoadedOnce] = useState(false); + const [editingRefreshFrequency, setEditingRefreshFrequency] = useState(false); + const [editingPruningFrequency, setEditingPruningFrequency] = useState(false); const { popup, setPopup } = usePopup(); const finishConnectorDeletion = useCallback(() => { @@ -90,6 +116,86 @@ function Main({ ccPairId }: { ccPairId: number }) { } }; + const handleRefreshEdit = async () => { + setEditingRefreshFrequency(true); + }; + + const handlePruningEdit = async () => { + setEditingPruningFrequency(true); + }; + + const handleRefreshSubmit = async ( + propertyName: string, + propertyValue: string + ) => { + const parsedRefreshFreq = parseInt(propertyValue, 10); + + if (isNaN(parsedRefreshFreq)) { + setPopup({ + message: "Invalid refresh frequency: must be an integer", + type: "error", + }); + return; + } + + try { + const response = await updateConnectorCredentialPairProperty( + ccPairId, + propertyName, + String(parsedRefreshFreq) + ); + if (!response.ok) { + throw new Error(await response.text()); + } + mutate(buildCCPairInfoUrl(ccPairId)); + setPopup({ + message: "Connector refresh frequency updated successfully", + type: "success", + }); + } catch (error) { + setPopup({ + message: "Failed to update connector refresh frequency", + type: "error", + }); + } + }; + + const handlePruningSubmit = async ( + propertyName: string, + propertyValue: string + ) => { + const parsedFreq = parseInt(propertyValue, 10); + + if (isNaN(parsedFreq)) { + setPopup({ + message: "Invalid pruning frequency: must be an integer", + type: "error", + }); + return; + } + + try { + const response = await updateConnectorCredentialPairProperty( + ccPairId, + propertyName, + String(parsedFreq) + ); + if (!response.ok) { + throw new Error(await response.text()); + } + mutate(buildCCPairInfoUrl(ccPairId)); + setPopup({ + message: "Connector pruning frequency updated successfully", + type: "success", + }); + } catch (error) { + setPopup({ + message: "Failed to update connector pruning frequency", + type: "error", + }); + } + }; + if (isLoading) { return ; } @@ -114,9 +220,35 @@ function Main({ ccPairId }: { ccPairId: number }) { refresh_freq: refreshFreq, indexing_start: indexingStart, } = ccPair.connector; + return ( <> {popup} + + {editingRefreshFrequency && ( + setEditingRefreshFrequency(false)} + /> + )} + + {editingPruningFrequency && ( + setEditingPruningFrequency(false)} + /> + )} + router.push("/admin/indexing/status")} /> @@ -213,6 +345,8 @@ function Main({ ccPairId }: { ccPairId: number }) { pruneFreq={pruneFreq} indexingStart={indexingStart} refreshFreq={refreshFreq} + onRefreshEdit={handleRefreshEdit} + onPruningEdit={handlePruningEdit} /> )} diff --git a/web/src/components/admin/connectors/Field.tsx b/web/src/components/admin/connectors/Field.tsx index 90d7629ac..51e221f21 100644 --- a/web/src/components/admin/connectors/Field.tsx +++ b/web/src/components/admin/connectors/Field.tsx @@ -209,7 +209,7 @@ export function TextFormField({ return (
-
+
{!removeLabel && (