Skip to content

Conversation

@liunaijie
Copy link
Member

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

@nielifeng nielifeng requested a review from Copilot April 17, 2025 01:19
Copy link
Contributor

Copilot AI left a 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 PR refactors the OpenMLDB connector options and parameters while updating the source to use CatalogTable for schema conversion. Key updates include:

  • Renaming configuration classes from OpenMldbConfig to OpenMldbSourceOptions and updating references accordingly.
  • Refactoring OpenMldbSource to use a constructor for initialization, converting schema to a CatalogTable, and updating the reader creation.
  • Adjustments in testing to reflect the new naming conventions for configuration options.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
seatunnel-connectors-v2/connector-openmldb/src/main/java/org/apache/seatunnel/connectors/seatunnel/openmldb/source/OpenMldbSourceFactory.java Updated option references and added createSource method implementation.
seatunnel-connectors-v2/connector-openmldb/src/main/java/org/apache/seatunnel/connectors/seatunnel/openmldb/source/OpenMldbSource.java Removed prepare() method and refactored schema conversion to return a CatalogTable.
seatunnel-connectors-v2/connector-openmldb/src/main/java/org/apache/seatunnel/connectors/seatunnel/openmldb/config/OpenMldbSourceOptions.java Renamed configuration class and updated its content accordingly.
seatunnel-connectors-v2/connector-openmldb/src/main/java/org/apache/seatunnel/connectors/seatunnel/openmldb/config/OpenMldbParameters.java Updated to work with the new SourceOptions.
seatunnel-ci-tools/src/test/java/org/apache/seatunnel/api/ConnectorOptionCheckTest.java Removed OpenMldbSourceOptions from the whitelist as part of the name change.
Comments suppressed due to low confidence (1)

seatunnel-connectors-v2/connector-openmldb/src/main/java/org/apache/seatunnel/connectors/seatunnel/openmldb/source/OpenMldbSource.java:143

  • The table name is hardcoded as "default" in the TableIdentifier. Consider making it configurable or more descriptive to better reflect the underlying data schema.
TableIdentifier.of("OpenMldb", openMldbParameters.getDatabase(), "default")

@hailin0 hailin0 merged commit d324fc5 into apache:dev Apr 25, 2025
6 checks passed
@liunaijie liunaijie deleted the improve/openmldb_options branch April 29, 2025 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants