Skip to content

Conversation

asifdxtreme
Copy link
Member

Auto Service Registration for spring-boot-starter-discovery

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 81.348% when pulling 8a60196 on asifhuawei:discovery-latest into 8771009 on ServiceComb:master.

<version>0.1.0-m2-SNAPSHOT</version>
</dependency>
<dependency>
<groupId>io.servicecomb</groupId>
Copy link
Member

@seanyinx seanyinx Jun 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the conflict resolving with master is not right. these dependencies were missing and removed on master.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, will fix it

<artifactId>java-chassis-core</artifactId>
<version>0.1.0-m2-SNAPSHOT</version>
</dependency>
<dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, will remove this

<groupId>io.servicecomb</groupId>
<artifactId>provider-springmvc</artifactId>
</dependency>
<dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grpc module was removed already

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay will remove this

if (null == this.client) {
this.client = RegistryClientFactory.getRegistryClient();
try {
this.client.init();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rebase onto branch JAV-30_discovery_client. this method shall not throw any exception.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay will remove try_catch block

</dependency>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-netflix-core</artifactId>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation is not right

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay

TestMgr.errors().clear();
}

@Test
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test doesn't have much meaning except ensured DiscoveryClient can be started up.

Nothing related to how services discovery with spring cloud integration and load balancing was tested.

We have to be sure we are able to forward requests to corresponding services from zuul with service center

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Written demo/IT code for:

  1. To test the discovery of particular micro-service
  2. To test the Zuul Proxy gateway feature to redirect the request to proper micro-service.

@asifdxtreme asifdxtreme force-pushed the discovery-latest branch 2 times, most recently from 67523ca to 2926655 Compare June 13, 2017 08:51
@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.448% when pulling 2926655 on asifhuawei:discovery-latest into 8771009 on ServiceComb:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 81.441% when pulling 2926655 on asifhuawei:discovery-latest into 8771009 on ServiceComb:master.

@@ -0,0 +1,51 @@
package io.servicecomb.springboot.starter.serviceregistry;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the license header here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

@@ -0,0 +1,28 @@
package io.servicecomb.springboot.starter.serviceregistry;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

License header.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

@Configuration
@ConditionalOnProperty(value = "spring.cloud.service-registry.enabled", matchIfMissing = true)
@AutoConfigureBefore(ServiceRegistryAutoConfiguration.class)
public class CseServiceRegistryAutoConfiguration {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid using CSE any more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay once this PR is merged I will re-factor the names and raise another MR immediately, right now the PR code size is big, re-merging/re-basing is adding extra effort.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.448% when pulling 3428b76 on asifhuawei:discovery-latest into 8771009 on ServiceComb:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 81.448% when pulling 687aef9 on asifhuawei:discovery-latest into c1c3866 on ServiceComb:master.

@seanyinx
Copy link
Member

could you add this <module>demo-spring-boot-discovery</module> to demo-parent to enable it for testing? I can't get it pass on my laptop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 80.709% when pulling e63d9b6 on asifhuawei:discovery-latest into c1c3866 on ServiceComb:master.

@asifdxtreme
Copy link
Member Author

Added the "spring-boot-starter-discovery" and "demo-spring-boot-starter-discovery" in parent pom

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 81.297% when pulling 3cfa5af on asifhuawei:discovery-latest into c1c3866 on ServiceComb:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.2%) to 80.246% when pulling 5343dd0 on asifhuawei:discovery-latest into c1c3866 on ServiceComb:master.

@asifdxtreme asifdxtreme force-pushed the discovery-latest branch 8 times, most recently from 378d881 to 8da52eb Compare June 14, 2017 11:14
@asifdxtreme asifdxtreme force-pushed the discovery-latest branch 6 times, most recently from c36d47b to 6707f17 Compare June 15, 2017 06:23
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 80.96% when pulling 6707f17 on asifhuawei:discovery-latest into c1c3866 on ServiceComb:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 81.035% when pulling e5882b9 on asifhuawei:discovery-latest into c1c3866 on ServiceComb:master.

@asifdxtreme asifdxtreme force-pushed the discovery-latest branch 2 times, most recently from f7b3fa1 to ce053f2 Compare June 15, 2017 09:26
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 81.079% when pulling b8c79ca on asifhuawei:discovery-latest into c1c3866 on ServiceComb:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 81.097% when pulling bf85e2b on asifhuawei:discovery-latest into c1c3866 on ServiceComb:master.

@seanyinx seanyinx merged commit 7fae760 into apache:master Jun 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants