Extend (legacy)meta server configuration. #1563
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1563 +/- ##
============================================
+ Coverage 48.62% 48.67% +0.05%
- Complexity 1895 1902 +7
============================================
Files 393 393
Lines 11495 11494 -1
Branches 1192 1194 +2
============================================
+ Hits 5589 5595 +6
+ Misses 5477 5471 -6
+ Partials 429 428 -1
Continue to review full report at Codecov.
|
|
Thanks! Normally, we think system properties have higher priority than system environment properties, is it ok to adjust the logic? Also, after adding system environment logic, the code looks a little complex, is it better to extract the get meta data logic as a standalone method? |
|
ok,I'll do some refactoring. |
| domains.put(Env.PRO, getMeteServerConf(env, prop, "pro_meta", "pro.meta")); | ||
| } | ||
|
|
||
| public String getMeteServerConf(Properties env, Properties prop, String sourceName, |
There was a problem hiding this comment.
getMeteServerConf -> getMetaServerAddress
| public String getMeteServerConf(Properties env, Properties prop, String sourceName, | ||
| String propName) { | ||
| // 1. Get from System Property. | ||
| String metaAddress = env.getProperty(sourceName); |
There was a problem hiding this comment.
how about System.getProperty(sourceName)?
| String metaAddress = env.getProperty(sourceName); | ||
| if (Strings.isNullOrEmpty(metaAddress)) { | ||
| // 2. Get from OS environment variable, which could not contain dot and is normally in UPPER case,like DEV_META. | ||
| metaAddress = getenv(sourceName.toUpperCase()); |
There was a problem hiding this comment.
System.getenv might be more clear :-)
|
Looks good to me, would you please stash the commits to one commit? |
Support to override meta server URL through environment variables.