You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/01/30 02:48:55 UTC
[GitHub] [incubator-shardingsphere] dongzl opened a new pull request #4109:
persist SchemaNames for getChildren.
dongzl opened a new pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109
For #3185 .
Changes proposed in this pull request:
- persist SchemaNames for getChildren.
- upgrade spring-boot version.
- fix server.yaml orchestration content.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] wgy8283335 commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
wgy8283335 commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373321547
##########
File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/main/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstance.java
##########
@@ -108,16 +106,12 @@ private String getDirectly(final String key) {
*/
@Override
public void persist(final String key, final String value) {
Review comment:
Whether should check update in schemas.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] tuohai666 commented on issue #4109:
persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
tuohai666 commented on issue #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#issuecomment-580102467
/run ci
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] wgy8283335 commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
wgy8283335 commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373322063
##########
File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
##########
@@ -41,9 +41,9 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
-public class NacosInstanceTest {
+public class NacosConfigInstanceTest {
Review comment:
NacosInstance or NacosConfigInstance?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373339945
##########
File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/main/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstance.java
##########
@@ -108,16 +106,12 @@ private String getDirectly(final String key) {
*/
@Override
public void persist(final String key, final String value) {
Review comment:
Update schema name info on `ConfigurationService`'s `persistShardingSchemaName` method.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] wgy8283335 merged pull request #4109:
persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
wgy8283335 merged pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373339289
##########
File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
##########
@@ -41,9 +41,9 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
-public class NacosInstanceTest {
+public class NacosConfigInstanceTest {
Review comment:
the same to 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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373310805
##########
File path: pom.xml
##########
@@ -79,7 +79,7 @@
<lombok.version>1.16.4</lombok.version>
<springframework.version>[4.3.6.RELEASE,5.0.0.M1)</springframework.version>
- <spring-boot.version>[1.5.0.RELEASE,2.0.0.M1)</spring-boot.version>
+ <spring-boot.version>[1.5.20.RELEASE,2.0.0.M1)</spring-boot.version>
Review comment:
Spring upgrade to https, the 1.5.0 version doesn't download correctly, so CI will be failure.
the dev branch upgrade the version to solve CI fail.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] wgy8283335 commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
wgy8283335 commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373310144
##########
File path: pom.xml
##########
@@ -79,7 +79,7 @@
<lombok.version>1.16.4</lombok.version>
<springframework.version>[4.3.6.RELEASE,5.0.0.M1)</springframework.version>
- <spring-boot.version>[1.5.0.RELEASE,2.0.0.M1)</spring-boot.version>
+ <spring-boot.version>[1.5.20.RELEASE,2.0.0.M1)</spring-boot.version>
Review comment:
Why change the version of spring-boot
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373339945
##########
File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/main/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstance.java
##########
@@ -108,16 +106,12 @@ private String getDirectly(final String key) {
*/
@Override
public void persist(final String key, final String value) {
Review comment:
I think the check update should for `overwrite`, if user set is true, we should overwrite value directly, if false, we do nothing.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373339376
##########
File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/main/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstance.java
##########
@@ -32,14 +30,18 @@
import org.apache.shardingsphere.orchestration.center.listener.DataChangedEvent;
import org.apache.shardingsphere.orchestration.center.listener.DataChangedEventListener;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.Executor;
+
/**
* The nacos instance for ConfigCenter.
*
* @author huangjian
* @author sunbufu
*/
@Slf4j
-public class NacosInstance implements ConfigCenter {
+public class NacosConfigInstance implements ConfigCenter {
Review comment:
the same to 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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] wgy8283335 commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
wgy8283335 commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373321910
##########
File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/main/resources/META-INF/services/org.apache.shardingsphere.orchestration.center.api.ConfigCenter
##########
@@ -15,4 +15,4 @@
# limitations under the License.
#
-org.apache.shardingsphere.orchestration.center.instance.NacosInstance
+org.apache.shardingsphere.orchestration.center.instance.NacosConfigInstance
Review comment:
Why rename nacos as "NacosConfigInstance"? What is the name for zookeeper, as it is used for config center, register center and distributed lock management?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373339243
##########
File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
##########
@@ -41,9 +41,9 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
-public class NacosInstanceTest {
+public class NacosConfigInstanceTest {
Review comment:
the same to 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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373339289
##########
File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/test/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstanceTest.java
##########
@@ -41,9 +41,9 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
-public class NacosInstanceTest {
+public class NacosConfigInstanceTest {
Review comment:
the same to 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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] wgy8283335 commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
wgy8283335 commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373328520
##########
File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/main/java/org/apache/shardingsphere/orchestration/center/instance/NacosConfigInstance.java
##########
@@ -32,14 +30,18 @@
import org.apache.shardingsphere.orchestration.center.listener.DataChangedEvent;
import org.apache.shardingsphere.orchestration.center.listener.DataChangedEventListener;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.Executor;
+
/**
* The nacos instance for ConfigCenter.
*
* @author huangjian
* @author sunbufu
*/
@Slf4j
-public class NacosInstance implements ConfigCenter {
+public class NacosConfigInstance implements ConfigCenter {
Review comment:
Why rename the nacos?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373843186
##########
File path: sharding-orchestration/sharding-orchestration-core/src/test/java/org/apache/shardingsphere/orchestration/internal/registry/config/service/ConfigurationServiceTest.java
##########
@@ -374,7 +373,7 @@ public void assertLoadProperties() {
@Test
public void assertGetAllShardingSchemaNames() {
- when(regCenter.getChildrenKeys("/test/config/schema")).thenReturn(Arrays.asList("sharding_db", "masterslave_db"));
+ when(regCenter.get("/test/config/schema")).thenReturn("sharding_db,masterslave_db");
Review comment:
I read nacos's issue list, It doesn't support.
https://github.com/alibaba/nacos/issues/1926
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] wgy8283335 commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
wgy8283335 commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373763763
##########
File path: sharding-orchestration/sharding-orchestration-core/src/test/java/org/apache/shardingsphere/orchestration/internal/registry/config/service/ConfigurationServiceTest.java
##########
@@ -374,7 +373,7 @@ public void assertLoadProperties() {
@Test
public void assertGetAllShardingSchemaNames() {
- when(regCenter.getChildrenKeys("/test/config/schema")).thenReturn(Arrays.asList("sharding_db", "masterslave_db"));
+ when(regCenter.get("/test/config/schema")).thenReturn("sharding_db,masterslave_db");
Review comment:
Instead of using mock, is there any possible solution to start a nacos server and put the value into 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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] dongzl commented on a change in pull
request #4109: persist SchemaNames for getChildren.
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #4109: persist SchemaNames for getChildren.
URL: https://github.com/apache/incubator-shardingsphere/pull/4109#discussion_r373339168
##########
File path: sharding-orchestration/sharding-orchestration-center/sharding-orchestration-center-nacos/src/main/resources/META-INF/services/org.apache.shardingsphere.orchestration.center.api.ConfigCenter
##########
@@ -15,4 +15,4 @@
# limitations under the License.
#
-org.apache.shardingsphere.orchestration.center.instance.NacosInstance
+org.apache.shardingsphere.orchestration.center.instance.NacosConfigInstance
Review comment:
nacos can as config center & register center, doesn't support distributed lock.
nacos is different for config center & register center, so we should have two class.
`NacosConfigInstance` is for Config center, `NacosRegistryInstance` will for register center.
----------------------------------------------------------------
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
With regards,
Apache Git Services