w41ter commented on code in PR #51199:
URL: https://github.com/apache/doris/pull/51199#discussion_r2157918476
##########
cloud/src/common/configbase.cpp:
##########
@@ -174,10 +216,10 @@ bool strtox(const std::string& valstr, std::string&
retval) {
}
template <typename T>
-bool convert(const std::string& value, T& retval) {
+bool convert(const std::string key, const std::string& value, T& retval) {
Review Comment:
```suggestion
bool convert(std::string_view key, const std::string& value, T& retval) {
```
##########
be/src/common/config.cpp:
##########
@@ -1745,7 +1787,7 @@ bool Properties::get_or_default(const char* key, const
char* defstr, T& retval,
}
rawval = valstr;
*is_retval_set = true;
- return convert(valstr, retval);
+ return convert(std::string(key), valstr, retval);
Review Comment:
```suggestion
return convert(key, valstr, retval);
```
##########
fe/fe-common/src/main/java/org/apache/doris/common/ConfigBase.java:
##########
@@ -193,19 +193,23 @@ public static String getConfValue(Field field) {
// config_key={CONFIG_VALUE}
// the "CONFIG_VALUE" should be replaced be env variable CONFIG_VALUE
private void replacedByEnv(Properties props) throws Exception {
- // pattern to match string like "{CONFIG_VALUE}"
- Pattern pattern = Pattern.compile("\\$\\{([^}]*)\\}");
+ // pattern to match string like "{CONFIG_VALUE}" or "CONFIG_VALUE"
+ Pattern pattern =
Pattern.compile("\\$(?:\\{([^}]+)\\}|([a-zA-Z_][a-zA-Z0-9_]*))");
for (String key : props.stringPropertyNames()) {
String value = props.getProperty(key);
Matcher m = pattern.matcher(value);
while (m.find()) {
- String envValue = System.getProperty(m.group(1));
- envValue = (envValue != null) ? envValue :
System.getenv(m.group(1));
+ String varName = (m.group(1) != null) ? m.group(1) :
m.group(2);
+ String envValue = System.getProperty(varName);
+ envValue = (envValue != null) ? envValue :
System.getenv(varName);
+
if (envValue != null) {
- value = value.replace("${" + m.group(1) + "}", envValue);
+ String originalPattern = (m.group(1) != null) ? "${" +
varName + "}" : "$" + varName;
+ value = value.replace(originalPattern, envValue);
} else {
throw new Exception("no such env variable: " + m.group(1));
}
+ LOG.info("replace conf ");
Review Comment:
It seems the LOG here is useless, so consider removing it. Or should we log
the key and value here?
##########
cloud/src/common/configbase.cpp:
##########
@@ -280,7 +322,7 @@ bool Properties::get_or_default(const char* key, const
char* defstr, T& retval,
valstr = it->second;
}
*is_retval_set = true;
- return convert(valstr, retval);
+ return convert(std::string(key), valstr, retval);
Review Comment:
```suggestion
return convert(key, valstr, retval);
```
##########
be/src/common/config.cpp:
##########
@@ -1669,10 +1711,10 @@ bool strtox(const std::string& valstr, std::string&
retval) {
}
template <typename T>
-bool convert(const std::string& value, T& retval) {
+bool convert(const std::string key, const std::string& value, T& retval) {
Review Comment:
```suggestion
bool convert(std::string_view key, const std::string& value, T& retval) {
```
##########
be/src/common/config.cpp:
##########
@@ -1564,23 +1564,65 @@ void splitkv(const std::string& s, std::string& k,
std::string& v) {
}
// replace env variables
-bool replaceenv(std::string& s) {
- std::size_t pos = 0;
- std::size_t start = 0;
- while ((start = s.find("${", pos)) != std::string::npos) {
- std::size_t end = s.find('}', start + 2);
- if (end == std::string::npos) {
- return false;
+bool replaceenv(std::string key, std::string& s) {
Review Comment:
```suggestion
bool replaceenv(std::string_view key, std::string& s) {
```
`std::string` will copy the content each time, `std::string_view` is just
holding the reference.
##########
cloud/src/common/configbase.cpp:
##########
@@ -69,23 +69,65 @@ void splitkv(const std::string& s, std::string& k,
std::string& v) {
}
// replace env variables
-bool replaceenv(std::string& s) {
- std::size_t pos = 0;
- std::size_t start = 0;
- while ((start = s.find("${", pos)) != std::string::npos) {
- std::size_t end = s.find("}", start + 2);
- if (end == std::string::npos) {
- return false;
+bool replaceenv(std::string key, std::string& s) {
Review Comment:
```suggestion
bool replaceenv(std::string_view key, std::string& s) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
