-
Notifications
You must be signed in to change notification settings - Fork 2k
[improve] typesense options #9398
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
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.
Pull Request Overview
This pull request refactors the configuration and option classes for the Typesense connector by renaming and consolidating configuration keys to use the new options classes (TypesenseBaseOptions, TypesenseSourceOptions, and TypesenseSinkOptions).
- Replaces deprecated config classes with the new options classes in the connector source, sink, and client code.
- Updates tests accordingly, including configuration for e2e tests and option validation in the CI tests.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
seatunnel-e2e/connector-typesense-e2e/TypesenseIT.java | Updated import and config mapping to use TypesenseBaseOptions. |
seatunnel-connectors-v2/connector-typesense/src/test/java/org/apache/seatunnel/connectors/seatunnel/typesense/serializer/TypesenseRowSerializerTest.java | Updated configuration keys from SinkConfig to the new options classes. |
seatunnel-connectors-v2/connector-typesense/src/main/java/org/apache/seatunnel/connectors/seatunnel/typesense/source/* | Updated source code to pull options from TypesenseBaseOptions and TypesenseSourceOptions. |
seatunnel-connectors-v2/connector-typesense/src/main/java/org/apache/seatunnel/connectors/seatunnel/typesense/sink/* | Updated sink options and configuration mapping to use TypesenseSinkOptions. |
seatunnel-connectors-v2/connector-typesense/src/main/java/org/apache/seatunnel/connectors/seatunnel/typesense/config/* | Renamed/configured option classes to reflect new naming conventions. |
seatunnel-ci-tools/src/test/java/org/apache/seatunnel/api/ConnectorOptionCheckTest.java | Adjusted test whitelist to reflect configuration changes. |
Comments suppressed due to low confidence (3)
seatunnel-connectors-v2/connector-typesense/src/main/java/org/apache/seatunnel/connectors/seatunnel/typesense/source/TypesenseSourceFactory.java:40
- [nitpick] Consider using a unified constant for the connector identity (e.g., TypesenseBaseOptions.CONNECTOR_IDENTITY) in source factories for consistency with the rest of the code.
return TypesenseSinkOptions.CONNECTOR_IDENTITY;
seatunnel-ci-tools/src/test/java/org/apache/seatunnel/api/ConnectorOptionCheckTest.java:192
- [nitpick] Confirm that the removal of TypesenseSourceOptions and TypesenseSinkOptions from the whitelist aligns with the updated configuration changes and does not cause unintended test failures.
whiteList.add("TypesenseSourceOptions") and whiteList.add("TypesenseSinkOptions") removal
seatunnel-connectors-v2/connector-typesense/src/main/java/org/apache/seatunnel/connectors/seatunnel/typesense/client/TypesenseClient.java:116
- [nitpick] Verify that using TypesenseSourceOptions.QUERY_BATCH_SIZE.defaultValue() correctly reflects the intended default batch size behavior for search operations.
return search(collection, query, offset, TypesenseSourceOptions.QUERY_BATCH_SIZE.defaultValue());
6971987
to
1031c2e
Compare
Purpose of this pull request
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide