Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
optimize: spring boot compatible with file.conf and registry.conf (#6811) #6828
base: 2.x
Are you sure you want to change the base?
optimize: spring boot compatible with file.conf and registry.conf (#6811) #6828
Changes from 3 commits
9203551
57aee15
da81dea
99e50a2
3515ea9
8837d11
e1c6769
f92343e
3140784
58c5867
4e2d72f
473ff27
cda9a19
d837845
44152ac
e4af159
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Why is this code being added?
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.
From an overall perspective, originalConfiguration.getConfig(rawDataId); is used to leverage getLatestConfig to fetch the configuration for rawDataId. After analysis, it indeed functionally covers getConfigFromSys. Therefore, it seems more appropriate to replace:
with:
instead of adding:
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.
Then why still keep
originalConfiguration.getConfigFromSys(rawDataId);
?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.
你目前的代码破坏了原本代码的目的,这段代码是为了先读环境变量和-D参数,然后再尝试读取spring中的配置,而你目前的改动变成了先读环境变量,在读配置中心,再读spring了,而这个SpringBootConfigurationProvider只是为了读取基本配置,配置中心中的配置有CURRENT_FILE_INSTANCE进行读取,不需要通过
ORIGIN_FILE_INSTANCE_REGISTRY
也就是SpringBootConfigurationProvider读取Your current code breaks the original purpose. This section of the code is intended to first read environment variables and -D parameters, and then attempt to read the configuration from Spring. However, with your modification, it now first reads from environment variables, then from the configuration center, and finally from Spring. The SpringBootConfigurationProvider is only meant to read basic configurations, and configurations in the configuration center are read via CURRENT_FILE_INSTANCE, so there’s no need to use ORIGIN_FILE_INSTANCE_REGISTRY, i.e., the SpringBootConfigurationProvider, to read them.
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.
Use SeataApplicationListener to set springConfigurableEnvironment,ensuring it's available during configuration retrieval.
This avoids the need for additional null checks and ensures proper loading timing.