Skip to content

Conversation

@thirsd
Copy link

@thirsd thirsd commented Mar 16, 2025

thanks to @hailin0 https://github.com/hailin0.

reference the design:
#8945 (comment)

Does this PR introduce any user-facing change?

yes
user can use ref config file to reuse config in job config file.

#8941

How was this patch tested?

  1. i have tested unset ref file .
  2. i have tested set ref file and use in mysql to console and sqlite to console.

Check list

thirsd added 4 commits March 15, 2025 20:20
…set path of ref_config, then you can use __st_config_ref_key__ in source\transform\sink to merge ref config.
source {
Jdbc {
__st_config_ref_key__ = "mysql_prod"
query="select * from department"
Copy link
Member

Choose a reason for hiding this comment

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

From the code, looks if the mysql_prod ref config also contains query parameter, it will overwrite the existed query parameter.

pluginConfig.withoutPath("__st_config_ref_key__").withFallback(refConfig);

Please add the priority for replacements when there are duplicate parameters.

Copy link
Author

Choose a reason for hiding this comment

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

got it

Copy link
Member

Choose a reason for hiding this comment

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

got it

We can just describe it in the document to let use know this thing. Replace behavior is good to me.

Copy link
Author

Choose a reason for hiding this comment

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

已经补充了配置优先级。

@liunaijie
Copy link
Member

Please add test case

Comment on lines +346 to +348
```hocon
# 定义 MySQL 连接配置
mysql_prod {
Copy link
Member

Choose a reason for hiding this comment

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

@thirsd Please describe in English here, thanks.

Comment on lines +357 to +359
# 定义 Kafka 连接配置
kafka_test {
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@davidzollo davidzollo added the First-time contributor First-time contributor label Mar 27, 2025
Map<String, Map<String, Object>> refMap = new HashMap<>();
// get map element in ref
for (String key : configMap.keySet()) {
Object ref_config = configMap.get(key);
Copy link
Contributor

Choose a reason for hiding this comment

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

we should name the variables in the camelcase style.

.withDescription("Define the worker where the job runs by tag");

public static Option<String> REF_PATH =
Options.key("__st_config_ref_path__")
Copy link
Contributor

Choose a reason for hiding this comment

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

__st_config_ref_path__ looks weird, how about naming it to config_ref_path?

Copy link
Member

Choose a reason for hiding this comment

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

Not surprising, he cannot conflict with normal attributes

@hailin0 hailin0 requested a review from Copilot March 31, 2025 05:17
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 introduces configuration reference support, allowing users to reuse shared configuration files in their job configurations. Key changes include adding a new environment option for reference config paths, merging plugin and reference configurations in the job parser, and updating tests and documentation to verify and explain the new behavior.

Reviewed Changes

Copilot reviewed 8 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
seatunnel-engine-core/src/main/java/org/apache/seatunnel/engine/core/parse/MultipleTableJobConfigParser.java Introduces refMaps field and merging logic for reference configs
seatunnel-engine-client/src/test/java/org/apache/seatunnel/engine/client/MultipleTableJobConfigParserTest.java Adds tests for parsing job configs using the ref config feature
seatunnel-core/seatunnel-core-starter/src/test/java/org/apache/seatunnel/core/starter/utils/ConfigShadeTest.java Extends tests to cover parsing and merging of reference configs
seatunnel-core/seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/utils/ConfigShadeUtils.java Adds processing branch to handle configuration references
seatunnel-api/src/main/java/org/apache/seatunnel/api/options/EnvCommonOptions.java Adds a new environment option for specifying the ref config file path
docs/* Updates both Chinese and English documentation to explain configuration reference functionality
Files not reviewed (4)
  • seatunnel-core/seatunnel-core-starter/src/test/resources/mysql_to_console_by_ref.conf: Language not supported
  • seatunnel-core/seatunnel-core-starter/src/test/resources/ref.conf: Language not supported
  • seatunnel-engine/seatunnel-engine-client/src/test/resources/fake_to_console_by_ref.conf: Language not supported
  • seatunnel-engine/seatunnel-engine-client/src/test/resources/ref.conf: Language not supported
Comments suppressed due to low confidence (1)

seatunnel-engine/seatunnel-engine-core/src/main/java/org/apache/seatunnel/engine/core/parse/MultipleTableJobConfigParser.java:164

  • [nitpick] The variable name 'RefPath' does not follow the Java camelCase naming convention. Consider renaming it to 'refPath'.
String RefPath = envOptions.getOptional(EnvCommonOptions.REF_PATH).orElse(null);

…e/seatunnel/engine/core/parse/MultipleTableJobConfigParser.java

Co-authored-by: Copilot <[email protected]>
@hailin0 hailin0 changed the title Ref config [Feature][Config] Support nested references to external files in configuration files Apr 2, 2025
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 adds support for nested external file references in configuration files so that common configurations can be reused across job config files. Key changes include:

  • Introducing a new configuration field (refMaps) and merging logic based on a dedicated reference key.
  • Adding tests to verify the new reference-merging behavior.
  • Updating documentation (both English and Chinese) and the API to include the new ref config options.

Reviewed Changes

Copilot reviewed 8 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
seatunnel-engine-core/src/main/java/org/apache/seatunnel/engine/core/parse/MultipleTableJobConfigParser.java Added refMaps field, updated constructors to load external config files, and implemented merge logic.
seatunnel-engine-client/src/test/java/org/apache/seatunnel/engine/client/MultipleTableJobConfigParserTest.java Introduced new test case for reference-based job configuration parsing.
seatunnel-core-starter/src/test/java/org/apache/seatunnel/core/starter/utils/ConfigShadeTest.java Added tests to validate reference configuration parsing.
seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/utils/ConfigShadeUtils.java Updated config processing logic to handle ref configurations.
seatunnel-api/src/main/java/org/apache/seatunnel/api/options/EnvCommonOptions.java Added a new option for the external reference config file path.
docs/zh/concept/config.md & docs/en/concept/config.md Updated documents to explain and illustrate configuration reference usage.
Files not reviewed (4)
  • seatunnel-core/seatunnel-core-starter/src/test/resources/mysql_to_console_by_ref.conf: Language not supported
  • seatunnel-core/seatunnel-core-starter/src/test/resources/ref.conf: Language not supported
  • seatunnel-engine/seatunnel-engine-client/src/test/resources/fake_to_console_by_ref.conf: Language not supported
  • seatunnel-engine/seatunnel-engine-client/src/test/resources/ref.conf: Language not supported
Comments suppressed due to low confidence (4)

seatunnel-engine/seatunnel-engine-core/src/main/java/org/apache/seatunnel/engine/core/parse/MultipleTableJobConfigParser.java:164

  • [nitpick] The variable name 'RefPath' starts with an uppercase letter, which is inconsistent with Java naming conventions. Consider renaming it to 'refPath'.
String RefPath = envOptions.getOptional(EnvCommonOptions.REF_PATH).orElse(null);

seatunnel-engine/seatunnel-engine-core/src/main/java/org/apache/seatunnel/engine/core/parse/MultipleTableJobConfigParser.java:198

  • [nitpick] Again, the variable 'RefPath' should be renamed to 'refPath' for consistency with standard Java naming conventions.
String RefPath = envOptions.getOptional(EnvCommonOptions.REF_PATH).orElse(null);

seatunnel-core/seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/utils/ConfigShadeUtils.java:213

  • [nitpick] The variable 'ref_dit' appears to be a typo; consider renaming it to 'refDict' for clarity and consistency.
Map<String, Object> ref_dit = new HashMap<>(RefConfig.size());

seatunnel-engine/seatunnel-engine-core/src/main/java/org/apache/seatunnel/engine/core/parse/MultipleTableJobConfigParser.java:200

  • In the second constructor, the variables parameter is passed as null to ConfigBuilder.of; verify if this is intended or if the required variables should be provided.
this.refMaps = ConfigBuilder.of(Paths.get(RefPath), null);

Config jobConfig = ConfigBuilder.of(Paths.get(jobConfigUrl.toURI()));

Config mysqlPluginConfig = jobConfig.getConfigList("source").get(0);
String refId = mysqlPluginConfig.getString("__st_config_ref_key__ ");
Copy link

Copilot AI Apr 18, 2025

Choose a reason for hiding this comment

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

There is an extra whitespace in the key string "st_config_ref_key ". This may lead to lookup failures; please remove the trailing space.

Suggested change
String refId = mysqlPluginConfig.getString("__st_config_ref_key__ ");
String refId = mysqlPluginConfig.getString("__st_config_ref_key__");

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.

@github-actions github-actions bot added the stale label Aug 17, 2025
@github-actions
Copy link

This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request.

@github-actions github-actions bot closed this Aug 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api core SeaTunnel core module document First-time contributor First-time contributor stale Zeta

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants