You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/05/21 06:57:09 UTC
[GitHub] [iotdb] wangchao316 opened a new pull request #3244: [IOTDB-1385] Extract the super user and password to the config
wangchao316 opened a new pull request #3244:
URL: https://github.com/apache/iotdb/pull/3244
Extract the super user and password to the config
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] LebronAl commented on a change in pull request #3244: [IOTDB-1385] Extract the super user and password to the config
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3244:
URL: https://github.com/apache/iotdb/pull/3244#discussion_r636791208
##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -831,3 +831,9 @@ timestamp_precision=ms
# time range for partitioning data inside each storage group, the unit is second
# Datatype: long
# partition_interval=604800
+
+# admin username, default is root
+#admin_name=root
Review comment:
```suggestion
# admin username, default is root
# Datatype: string
# admin_name=root
```
##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -831,3 +831,9 @@ timestamp_precision=ms
# time range for partitioning data inside each storage group, the unit is second
# Datatype: long
# partition_interval=604800
+
+# admin username, default is root
+#admin_name=root
+
+# admin password, default is root
+#admin_password=root
Review comment:
```suggestion
# admin password, default is root
# Datatype: string
# admin_password=root
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] wangchao316 commented on pull request #3244: [IOTDB-1385] Extract the super user and password to the config
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on pull request #3244:
URL: https://github.com/apache/iotdb/pull/3244#issuecomment-845755957
@LebronAl @neuyilan @mychaow hi, could you please review?
Thanks
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] LebronAl merged pull request #3244: [IOTDB-1385] Extract the super user and password to the config
Posted by GitBox <gi...@apache.org>.
LebronAl merged pull request #3244:
URL: https://github.com/apache/iotdb/pull/3244
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] wangchao316 commented on a change in pull request #3244: [IOTDB-1385] Extract the super user and password to the config
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #3244:
URL: https://github.com/apache/iotdb/pull/3244#discussion_r636762995
##########
File path: server/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java
##########
@@ -701,19 +701,23 @@ private void loadProps() {
}
conf.setAuthorizerProvider(
- properties.getProperty(
- "authorizer_provider_class",
- "org.apache.iotdb.db.auth.authorizer.LocalFileAuthorizer"));
+ properties.getProperty("authorizer_provider_class", conf.getAuthorizerProvider()));
// if using org.apache.iotdb.db.auth.authorizer.OpenIdAuthorizer, openID_url is needed.
- conf.setOpenIdProviderUrl(properties.getProperty("openID_url", ""));
+ conf.setOpenIdProviderUrl(properties.getProperty("openID_url", conf.getOpenIdProviderUrl()));
Review comment:
Thanks xinyu
do it.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] LebronAl commented on a change in pull request #3244: [IOTDB-1385] Extract the super user and password to the config
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3244:
URL: https://github.com/apache/iotdb/pull/3244#discussion_r636754164
##########
File path: server/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java
##########
@@ -701,19 +701,23 @@ private void loadProps() {
}
conf.setAuthorizerProvider(
- properties.getProperty(
- "authorizer_provider_class",
- "org.apache.iotdb.db.auth.authorizer.LocalFileAuthorizer"));
+ properties.getProperty("authorizer_provider_class", conf.getAuthorizerProvider()));
// if using org.apache.iotdb.db.auth.authorizer.OpenIdAuthorizer, openID_url is needed.
- conf.setOpenIdProviderUrl(properties.getProperty("openID_url", ""));
+ conf.setOpenIdProviderUrl(properties.getProperty("openID_url", conf.getOpenIdProviderUrl()));
conf.setEnablePartition(
Boolean.parseBoolean(
- properties.getProperty("enable_partition", conf.isEnablePartition() + "")));
+ properties.getProperty(
Review comment:
Hi, As we set these field here, it's better to remove commented code in 599-611 lines
##########
File path: server/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java
##########
@@ -701,19 +701,23 @@ private void loadProps() {
}
conf.setAuthorizerProvider(
- properties.getProperty(
- "authorizer_provider_class",
- "org.apache.iotdb.db.auth.authorizer.LocalFileAuthorizer"));
+ properties.getProperty("authorizer_provider_class", conf.getAuthorizerProvider()));
// if using org.apache.iotdb.db.auth.authorizer.OpenIdAuthorizer, openID_url is needed.
- conf.setOpenIdProviderUrl(properties.getProperty("openID_url", ""));
+ conf.setOpenIdProviderUrl(properties.getProperty("openID_url", conf.getOpenIdProviderUrl()));
Review comment:
The default value for `openIdProviderUrl` is `null`, which is different with `""`.
It seems that here will throw an exception:
##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -831,3 +831,9 @@ timestamp_precision=ms
# time range for partitioning data inside each storage group, the unit is second
# Datatype: long
# partition_interval=604800
+
+# admin username, default is root
+admin_name=root
Review comment:
We'd better keep these comments in a consistent format.
Such as
```
# admin username, default is root
# Datatype: string
# admin_name=root
```
##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -831,3 +831,9 @@ timestamp_precision=ms
# time range for partitioning data inside each storage group, the unit is second
# Datatype: long
# partition_interval=604800
+
+# admin username, default is root
+admin_name=root
+
+# admin password, default is root
+admin_password=root
Review comment:
same as above
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] LebronAl commented on a change in pull request #3244: [IOTDB-1385] Extract the super user and password to the config
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3244:
URL: https://github.com/apache/iotdb/pull/3244#discussion_r636792769
##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -831,3 +831,13 @@ timestamp_precision=ms
# time range for partitioning data inside each storage group, the unit is second
# Datatype: long
# partition_interval=604800
+
+# admin username, default is root
Review comment:
```suggestion
```
##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -831,3 +831,13 @@ timestamp_precision=ms
# time range for partitioning data inside each storage group, the unit is second
# Datatype: long
# partition_interval=604800
+
+# admin username, default is root
+# admin username, default is root
+# Datatype: string
+# admin_name=root
+
+# admin password, default is root
Review comment:
```suggestion
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] wangchao316 commented on a change in pull request #3244: [IOTDB-1385] Extract the super user and password to the config
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #3244:
URL: https://github.com/apache/iotdb/pull/3244#discussion_r636763106
##########
File path: server/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java
##########
@@ -701,19 +701,23 @@ private void loadProps() {
}
conf.setAuthorizerProvider(
- properties.getProperty(
- "authorizer_provider_class",
- "org.apache.iotdb.db.auth.authorizer.LocalFileAuthorizer"));
+ properties.getProperty("authorizer_provider_class", conf.getAuthorizerProvider()));
// if using org.apache.iotdb.db.auth.authorizer.OpenIdAuthorizer, openID_url is needed.
- conf.setOpenIdProviderUrl(properties.getProperty("openID_url", ""));
+ conf.setOpenIdProviderUrl(properties.getProperty("openID_url", conf.getOpenIdProviderUrl()));
conf.setEnablePartition(
Boolean.parseBoolean(
- properties.getProperty("enable_partition", conf.isEnablePartition() + "")));
+ properties.getProperty(
Review comment:
done
##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -831,3 +831,9 @@ timestamp_precision=ms
# time range for partitioning data inside each storage group, the unit is second
# Datatype: long
# partition_interval=604800
+
+# admin username, default is root
+admin_name=root
Review comment:
done
##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -831,3 +831,9 @@ timestamp_precision=ms
# time range for partitioning data inside each storage group, the unit is second
# Datatype: long
# partition_interval=604800
+
+# admin username, default is root
+admin_name=root
+
+# admin password, default is root
+admin_password=root
Review comment:
done
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] LebronAl commented on a change in pull request #3244: [IOTDB-1385] Extract the super user and password to the config
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3244:
URL: https://github.com/apache/iotdb/pull/3244#discussion_r636755993
##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -831,3 +831,9 @@ timestamp_precision=ms
# time range for partitioning data inside each storage group, the unit is second
# Datatype: long
# partition_interval=604800
+
+# admin username, default is root
+admin_name=root
Review comment:
We'd better keep these parameters in a consistent format.
Such as
```
# admin username, default is root
# Datatype: string
# admin_name=root
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] LebronAl commented on a change in pull request #3244: [IOTDB-1385] Extract the super user and password to the config
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3244:
URL: https://github.com/apache/iotdb/pull/3244#discussion_r636752304
##########
File path: server/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java
##########
@@ -701,19 +701,23 @@ private void loadProps() {
}
conf.setAuthorizerProvider(
- properties.getProperty(
- "authorizer_provider_class",
- "org.apache.iotdb.db.auth.authorizer.LocalFileAuthorizer"));
+ properties.getProperty("authorizer_provider_class", conf.getAuthorizerProvider()));
// if using org.apache.iotdb.db.auth.authorizer.OpenIdAuthorizer, openID_url is needed.
- conf.setOpenIdProviderUrl(properties.getProperty("openID_url", ""));
+ conf.setOpenIdProviderUrl(properties.getProperty("openID_url", conf.getOpenIdProviderUrl()));
Review comment:
The default value for `openIdProviderUrl` is `null`, which is different from `""`.
It seems that here will throw an exception:
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org