Skip to content

Conversation

@Yuukadesu
Copy link
Contributor

Purpose

support MilvusSink ,and this Sink support FloatVector and BinaryVector now

Remarks

PR introduces (a) breaking change(s): <yes/no>

PR introduces (a) deprecation(s): <yes/no>

@github-actions github-actions bot added dependencies Pull requests that update a dependency file java Pull requests that update Java code pipeline elements Relates to pipeline elements backend Everything that is related to the StreamPipes backend documentation Everything related to documentation labels Apr 8, 2025
Copy link
Member

@dominikriemer dominikriemer left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your PR! I added some suggestions how to improve the PR before merging it.

IndexParam indexParam;
CreateCollectionReq.CollectionSchema collectionSchema;

public static final String BYTE = "http://www.w3.org/2001/XMLSchema#byte";
Copy link
Member

Choose a reason for hiding this comment

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

There is an Xsd class in org.apache.streampipes.vocabulary which provide these variables, could you reuse these?

client.createDatabase(createDatabaseReq);
client.useDatabase(dbName);
} catch (Exception ignored) {
//todo add log
Copy link
Member

Choose a reason for hiding this comment

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

Can you add some logging here? You can throw a SpRuntimeException so that any problems when creating the database connection will show up in the UI.

DescribeCollectionReq describeCollectionReq = DescribeCollectionReq.builder()
.collectionName(this.collectionName)
.build();
System.out.println("collection name" + this.collectionName);
Copy link
Member

Choose a reason for hiding this comment

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

System.out can be replaced by logging

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestions! I will use these suggestions to modify my code.

@tenthe
Copy link
Contributor

tenthe commented Apr 10, 2025

@Yuukadesu thanks a lot for your PR

@tenthe tenthe merged commit 33d046c into apache:dev Apr 10, 2025
22 checks passed
@dominikriemer dominikriemer added this to the 0.98.0 milestone Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Everything that is related to the StreamPipes backend dependencies Pull requests that update a dependency file documentation Everything related to documentation java Pull requests that update Java code pipeline elements Relates to pipeline elements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants