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

DBZ-8634 Milvus sink #156

Merged
merged 9 commits into from
Mar 31, 2025
Merged

DBZ-8634 Milvus sink #156

merged 9 commits into from
Mar 31, 2025

Conversation

jpechane
Copy link
Contributor

@jpechane jpechane commented Feb 6, 2025

No description provided.

@jpechane jpechane changed the title DRAFT: DBZ-8634 Milvus sink WIP: DBZ-8634 Milvus sink Feb 6, 2025
vjuranek
vjuranek reviewed Feb 6, 2025
View reviewed changes
Copy link
Member

@vjuranek vjuranek left a comment

Choose a reason for hiding this comment

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

I only skimmed through the PR, so maybe I'm missing something, but why is all the Pulsar and related stuff (bookkeeper, zookeeper) needed? Or all this stuff will be removed (as a remnant from copying Pulsar module)?

@jpechane jpechane force-pushed the DBZ-8634 branch 2 times, most recently from 7f29ce5 to d4426bb Compare March 28, 2025 09:19
@jpechane jpechane marked this pull request as ready for review March 28, 2025 09:23
@jpechane jpechane requested a review from vjuranek March 28, 2025 09:23
@jpechane
Copy link
Contributor Author

@vjuranek The PR was cleaned and the implementation is at MVP level. Please take a look.
Later on, when your SMT is finished we can add another test that would take a table without vector column and add the vector via the vectorization SMT.

@jpechane jpechane changed the title WIP: DBZ-8634 Milvus sink DBZ-8634 Milvus sink Mar 28, 2025
@jpechane jpechane mentioned this pull request Mar 28, 2025
Open
vjuranek
vjuranek reviewed Mar 31, 2025
View reviewed changes
Copy link
Member

@vjuranek vjuranek left a comment

Choose a reason for hiding this comment

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

Few minor comments, but over LGTM, nice work!

jpechane
jpechane commented Mar 31, 2025
View reviewed changes
Copy link
Contributor Author

@jpechane jpechane left a comment

Choose a reason for hiding this comment

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

@vjuranek Please check the answers

@jpechane jpechane requested a review from vjuranek March 31, 2025 13:08
vjuranek
vjuranek approved these changes Mar 31, 2025
View reviewed changes
Copy link
Member

@vjuranek vjuranek left a comment

Choose a reason for hiding this comment

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

LGTM (once the compilation failure is fixed)

jpechane and others added 2 commits March 31, 2025 16:27
@jpechane @vjuranek
DBZ-8634 Fix scope
87c7301 
Co-authored-by: Vojtěch Juránek <[email protected]>
@jpechane @vjuranek
DBZ-8634 Minor fixes from code review
fc47c8a 
Co-authored-by: Vojtěch Juránek <[email protected]>
@jpechane jpechane merged commit 5102730 into debezium:main Mar 31, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vjuranek vjuranek vjuranek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jpechane @vjuranek