-
Notifications
You must be signed in to change notification settings - Fork 2k
[Feature] Support connection config, and use conn_id to replace conn info in job config #8945
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
You can initiate a discussion on the developer mailing list, which requires majority consensus. |
We should have a separate module to maintain configuration items, which can be a third-party tool, such as apache gravitino.Or of course you can maintain it yourself, as you are doing now. First of all, you need a design plan. |
已经修复问题。 目前采用方式为增加--connect参数,通过指定connection对应的file文件,从而在job的config中若存在conn_id的信息,则会使用connection中的配置进行代替。 |
We can add a separate module to support it.It can support major catalogs, such as apache gravitino or seatunnel's own catalog management tool. In terms of usage, we can not add --conn but process it through env environment variables. |
Any suggestions? @Hisoka-X @liugddx common.conf
job_example.conf
parsed config result
|
+1. |
Why do we need to resubmit the PR? The context of the review will be lost. |
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 PR introduces support for connection configuration by adding a new connection config file that allows users to replace connection details in job configurations with a connection ID. It updates parsers, execution environment interfaces, client commands, and constants to integrate the new functionality.
- Add a new connection configuration field and merging logic in the job config parser.
- Update various client and command classes to pass and handle a connection config file path.
- Extend configuration utilities and constants to support the connection config.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
seatunnel-engine/seatunnel-engine-core/src/main/java/org/apache/seatunnel/engine/core/parse/MultipleTableJobConfigParser.java | Added connection config field and merge logic for replacing conn_id with connection details. |
seatunnel-engine/seatunnel-engine-client/src/main/java/org/apache/seatunnel/engine/client/job/ClientJobExecutionEnvironment.java | Added new constructor parameter and field for connection file path. |
seatunnel-engine/seatunnel-engine-client/src/main/java/org/apache/seatunnel/engine/client/SeaTunnelClientInstance.java | Added interface methods to support passing a connection config path. |
seatunnel-engine/seatunnel-engine-client/src/main/java/org/apache/seatunnel/engine/client/SeaTunnelClient.java | Extended client methods to include connection config path in execution context creation. |
seatunnel-core/seatunnel-starter/src/main/java/org/apache/seatunnel/core/starter/seatunnel/command/ClientExecuteCommand.java | Added logic to process and verify the connection config file path parameter from command-line arguments. |
seatunnel-core/seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/utils/ConfigShadeUtils.java | Updated configuration processing to handle connection configuration options. |
seatunnel-core/seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/command/AbstractCommandArgs.java | Introduced a new parameter for connection configuration file. |
seatunnel-common/src/main/java/org/apache/seatunnel/common/Constants.java | Introduced a constant for connection configuration. |
Comments suppressed due to low confidence (1)
seatunnel-core/seatunnel-starter/src/main/java/org/apache/seatunnel/core/starter/seatunnel/command/ClientExecuteCommand.java:141
- [nitpick] Consider renaming the variable 'str_conn_path' to 'connFilePath' or 'connFileAbsolutePath' for improved consistency and readability with other naming conventions.
String str_conn_path = null;
Map<String, Object> conn_dit = new HashMap<>(connect.size()); | ||
for (String sensitiveOption : sensitiveOptions) { | ||
conn_dit.computeIfPresent(sensitiveOption, processFunction); | ||
} | ||
connect = conn_dit; |
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.
[nitpick] The variable name 'conn_dit' appears to be a typo; consider renaming it to 'conn_dict' for clarity.
Map<String, Object> conn_dit = new HashMap<>(connect.size()); | |
for (String sensitiveOption : sensitiveOptions) { | |
conn_dit.computeIfPresent(sensitiveOption, processFunction); | |
} | |
connect = conn_dit; | |
Map<String, Object> conn_dict = new HashMap<>(connect.size()); | |
for (String sensitiveOption : sensitiveOptions) { | |
conn_dict.computeIfPresent(sensitiveOption, processFunction); | |
} | |
connect = conn_dict; |
Copilot uses AI. Check for mistakes.
this commit is for #8941
Purpose of this pull request
Does this PR introduce any user-facing change?
user can add -conn or --connect to set Connect Config file.
connections.conf
sqlite_to_console.conf:
How was this patch tested?
no conn_id example
sqlite example
mysql example
Check list
New License Guide
release-note
.