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

Jump to bottom

[KWSearch] Double Confluence parent IDs #9274

Merged
merged 9 commits into from
Dec 12, 2024
Merged

[KWSearch] Double Confluence parent IDs #9274

merged 9 commits into from
Dec 12, 2024

Conversation

aubin-tchoi
Copy link
Contributor

@aubin-tchoi aubin-tchoi commented Dec 11, 2024 •
edited
Loading

Description

  • Part of #9255
  • First step of the plan described in #9069:
    • double the parent IDs passed by connectors
  • After merging this PR we will run the migration that backfills the parents in data_source_documents and qdrant and we will have all parents doubled in core (postgres + qdrant).

Next steps:

  • run the backfill that x2 everything
  • in connectors use the new parents in contentNodes
  • backfill agent_data_source_configurations
  • in connectors start upserting only the new parents
  • cleanup core

Risk

  • Introduce inconsistencies in data.

Deploy Plan

  • Deploy connectors.
  • Snapshot the Confluence datasources.
  • Run the migration on core.

@aubin-tchoi aubin-tchoi self-assigned this Dec 11, 2024
philipperolet
philipperolet approved these changes Dec 12, 2024
View reviewed changes
Copy link
Contributor

@philipperolet philipperolet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 (if you're confident in your grepping skills on parents 😄 )

connectors/src/connectors/confluence/lib/internal_ids.ts
@@ -27,3 +32,13 @@ export function isConfluenceInternalPageId(
): internalId is `${ConfluenceInternalIdPrefix.Page}${string}` {
return internalId.startsWith(ConfluenceInternalIdPrefix.Page);
}

export function convertInternalIdToDocumentId(internalId: string): string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the intent to merge the functions after the internal id is removed? (aka delete getId fns)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this is the plan, I think we can do it after all the doubling up and migrating is done

@aubin-tchoi aubin-tchoi merged commit 7c0707a into main Dec 12, 2024
3 checks passed
@aubin-tchoi aubin-tchoi deleted the confluence-align-ids branch December 12, 2024 09:28
@aubin-tchoi aubin-tchoi mentioned this pull request Dec 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philipperolet philipperolet philipperolet approved these changes

Assignees

@aubin-tchoi aubin-tchoi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aubin-tchoi @philipperolet