-
Notifications
You must be signed in to change notification settings - Fork 27
feat: support mongo connector #471
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
base: main
Are you sure you want to change the base?
Conversation
@sowen1023 please help to review this PR |
- Add missing 'if' keyword in conditional statement at line 257 - Fixes parsing error that prevented tests from running Co-Authored-By: windWheel <[email protected]>
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.
Testing...
count := 0 | ||
maxBatchSize := 1000 // Prevent memory overflow | ||
|
||
for cursor.Next(context.Background()) && count < maxBatchSize { |
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.
Maybe we can use len(documents)
instead of count
to eliminate the separate variable?
return nil | ||
} | ||
|
||
func (p *Plugin) shouldStop() bool { |
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.
shouldStop
is never used and should not be used like this.
@undertaker86001 any updates of this PR? |
I'm resolving a conflict. |
@@ -17,6 +17,10 @@ export const Types = { | |||
RSS: 'rss', | |||
S3: 's3', | |||
Yuque: 'yuque', | |||
S3: 's3', |
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.
duplicated key S3
here
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.
also Confluence
and NetworkDrive
|
I don’t think we need a dedicated sync manager or strategy specifically for MongoDB only. If such functionality is required, it should be built on top of the existing connector task infrastructure, and it is target for long term roadmap. For consistency and maintainability, why not keep it simple and align with how the other connectors work? |
OK, I'll make the changes |
What does this PR do
Related to issue-456
Rationale for this change
Standards checklist