-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
Multiple environmental isolation #1075
Conversation
merge for name change
Codecov Report
@@ Coverage Diff @@
## develop #1075 +/- ##
=============================================
- Coverage 41.87% 41.85% -0.02%
Complexity 1361 1361
=============================================
Files 243 243
Lines 10118 10118
Branches 1325 1325
=============================================
- Hits 4237 4235 -2
Misses 5333 5333
- Partials 548 550 +2
Continue to review full report at Codecov.
|
Good! |
/** | ||
* The constant FILE_INSTANCE. | ||
*/ | ||
public static final Configuration FILE_INSTANCE = new FileConfiguration(REGISTRY_CONF); | ||
private static final String ENV_VALUE = System.getProperty("env"); |
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.
maybe you can provide a util class to set or resolve the env value
default it can fetch from system properties
when users use spring ,maybe they want to config from the application.properties ,so they can invoke your util method instead of use System.setProperty
or pass by -D
@leizhiyuan To read configs from application,it's better to create a module separately to do that.like seata-spring-boot-starter. |
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.
LGTM
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.
LGTM
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.
There are conflict files.
This kind of configuration isolation is acceptable on the server side, but may not be so good on the client side, such as spring-boot. |
I think an environment variable can be added. The name is SEATA_CONFIG_ENV. Before loading the configuration file, both the server and the client should read the environment variable of the current System (using system.getenv).If it's not blank then load the corresponding configuration files, such as registry-dev.conf and file-dev.conf, where dev is the value read from the environment variable. |
Good idea. |
config/seata-config-core/src/main/java/io/seata/config/ConfigurationFactory.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
private static final Configuration DEFAULT_FILE_INSTANCE = new FileConfiguration(REGISTRY_CONF_PREFIX + REGISTRY_CONF_SUFFIX); | ||
public static final Configuration CURRENT_FILE_INSTANCE = (ENV_VALUE == null || "default".equals(ENV_VALUE)) ? DEFAULT_FILE_INSTANCE : new FileConfiguration(REGISTRY_CONF_PREFIX + "-" + ENV_VALUE + REGISTRY_CONF_SUFFIX); |
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.
it's better to define a constant for "default".
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.
Thanks.LGTM.
由于前期Seata社区治理规范问题部分代码作者未签署CLA,可能引发社区知识产权风险问题。请所有在Seata社区贡献过代码(包含:主项目、官网、samples和多语言项目等)的 contributor 帮忙在这个链接登录github账号签署相应的开发者CLA,https://cla-assistant.io/seata/seata 。2023.1.31 未签署CLA的代码将会被rewrite,拜托大家帮忙签一下。 |
Switch environments by specifying env environment variables, let seata use different environment configurations.
example:
past
+file.conf
+registry.conf
now
+file-dev.conf
+registry-dev.conf
+file.conf
+registry.conf
Use -Denv=dev to recognize the suffix, and choose registry-dev.conf configuration.
when -Denv=default or null, use registry.conf.
when -Denv=unknown, use NULL configuration, at this point some objects will report an error due to missing configuration.