-
Notifications
You must be signed in to change notification settings - Fork 115
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.
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
Slack Sync updates slack docs parents Field #1295
Conversation
@@ -559,7 +559,7 @@ export async function syncThread( | |||
documentUrl: sourceUrl, | |||
timestampMs: createdAt, | |||
tags, | |||
parents: [], | |||
parents: [channelId], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance we could guarantee that we use the same Id here and in the connector ressource internal Id?
I worry we change one and not the other?
Maybe it;'s not possible and we leave a comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per slack thread, the convention is now to use external ids directly everywhere, so it is legit to leave it as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
group: ["documentId", "channelId"], | ||
}); | ||
// update all parents fields for all pages and databases by chunks of 128 | ||
const chunkSize = 128; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a lot of connections, maybe 32?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will update in all 3 migrations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit worried by
Based on the similar Notion PR for review simplicity (there is a common change)
Much, much, much simpler than Notion
Same migration plan