Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quick fix #4389

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
+15 −47
Draft

Quick fix #4389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f280ba
quick fix
pablonyx Mar 29, 2025
edaca1f
update
pablonyx Mar 29, 2025
6167662
nit
pablonyx Mar 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 5 additions & 23 deletions backend/onyx/background/celery/tasks/connector_deletion/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,8 @@
from onyx.configs.constants import OnyxRedisSignals
from onyx.db.connector import fetch_connector_by_id
from onyx.db.connector_credential_pair import add_deletion_failure_message
from onyx.db.connector_credential_pair import (
delete_connector_credential_pair__no_commit,
)
from onyx.db.connector_credential_pair import get_connector_credential_pair_from_id
from onyx.db.connector_credential_pair import get_connector_credential_pairs
from onyx.db.document import (
delete_all_documents_by_connector_credential_pair__no_commit,
)
from onyx.db.document import get_document_ids_for_connector_credential_pair
from onyx.db.document_set import delete_document_set_cc_pair_relationship__no_commit
from onyx.db.engine import get_session_with_current_tenant
Expand Down Expand Up @@ -449,27 +443,15 @@ def monitor_connector_deletion_taskset(
connector_id_to_delete = cc_pair.connector_id
credential_id_to_delete = cc_pair.credential_id

# Explicitly delete document by connector credential pair records before deleting the connector
# This is needed because connector_id is a primary key in that table and cascading deletes won't work
delete_all_documents_by_connector_credential_pair__no_commit(
db_session=db_session,
connector_id=connector_id_to_delete,
credential_id=credential_id_to_delete,
)
# No need to explicitly delete DocumentByConnectorCredentialPair records anymore
# as we have proper cascade relationships set up in the models

# Flush to ensure document deletion happens before connector deletion
# Flush to ensure all operations happen in sequence
db_session.flush()

# Expire the cc_pair to ensure SQLAlchemy doesn't try to manage its state
# related to the deleted DocumentByConnectorCredentialPair during commit
db_session.expire(cc_pair)
# Delete the cc-pair directly
db_session.delete(cc_pair)

# finally, delete the cc-pair
delete_connector_credential_pair__no_commit(
db_session=db_session,
connector_id=connector_id_to_delete,
credential_id=credential_id_to_delete,
)
# if there are no credentials left, delete the connector
connector = fetch_connector_by_id(
db_session=db_session,
Expand Down
22 changes: 0 additions & 22 deletions backend/onyx/db/document.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,28 +555,6 @@ def delete_documents_by_connector_credential_pair__no_commit(
db_session.execute(stmt)


def delete_all_documents_by_connector_credential_pair__no_commit(
db_session: Session,
connector_id: int,
credential_id: int,
) -> None:
"""Deletes all document by connector credential pair entries for a specific connector and credential.
This is primarily used during connector deletion to ensure all references are removed
before deleting the connector itself. This is crucial because connector_id is part of the
primary key in DocumentByConnectorCredentialPair, and attempting to delete the Connector
would otherwise try to set the foreign key to NULL, which fails for primary keys.

NOTE: Does not commit the transaction, this must be done by the caller.
"""
stmt = delete(DocumentByConnectorCredentialPair).where(
and_(
DocumentByConnectorCredentialPair.connector_id == connector_id,
DocumentByConnectorCredentialPair.credential_id == credential_id,
)
)
db_session.execute(stmt)


def delete_documents__no_commit(db_session: Session, document_ids: list[str]) -> None:
db_session.execute(delete(DbDocument).where(DbDocument.id.in_(document_ids)))

Expand Down
12 changes: 10 additions & 2 deletions backend/onyx/db/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,11 @@ class Connector(Base):
)
documents_by_connector: Mapped[
list["DocumentByConnectorCredentialPair"]
] = relationship("DocumentByConnectorCredentialPair", back_populates="connector")
] = relationship(
"DocumentByConnectorCredentialPair",
back_populates="connector",
cascade="all, delete-orphan",
)

# synchronize this validation logic with RefreshFrequencySchema etc on front end
# until we have a centralized validation schema
Expand Down Expand Up @@ -748,7 +752,11 @@ class Credential(Base):
)
documents_by_credential: Mapped[
list["DocumentByConnectorCredentialPair"]
] = relationship("DocumentByConnectorCredentialPair", back_populates="credential")
] = relationship(
"DocumentByConnectorCredentialPair",
back_populates="credential",
cascade="all, delete-orphan",
)

user: Mapped[User | None] = relationship("User", back_populates="credentials")

Expand Down