-
Notifications
You must be signed in to change notification settings - Fork 114
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
[MS connector] Tweaks on incremental sync #6357
Conversation
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.
LGTM 👍🏼
? null | ||
: getParentReferenceInternalId(driveItem.parentReference); | ||
|
||
// check if we |
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.
nit
: missing something here I guess 😺
dataSourceConfig: DataSourceConfig; | ||
startSyncTs: number; | ||
}) { | ||
const children = await folder.fetchChildren(); |
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.
nit
: I added an optional argument where you can pass a list of nodeTypes
to filter on. Not gonna change much but you could do:
const children = await folder.fetchChildren(["file", "folder"]);
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.
In that case, i'd rather fetch all children and have the code err if they're not files or folders --- since it should not happen (not sure i'm clear 😅 happy to clarify irl)
4baa156
to
b35645a
Compare
* fix: parentInternalId should be null by default, and always null for roots * handle 410 require resync error * update parents on folder movement * fix movement detection * lint * clean com
Description
Fixes a few remaining issues in incremental sync:
Fixes #6289
Risk
na