Skip to content

Conversation

@acsukesh
Copy link
Contributor

…l/EnableAutoDiscovery

Signed-off-by: sukesh a c sukeshac@huawei.com

…l/EnableAutoDiscovery

Signed-off-by: sukesh a c <sukeshac@huawei.com>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-context</artifactId>
</dependency>
<groupId>org.springframework.cloud</groupId>
Copy link
Member

Choose a reason for hiding this comment

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

please fix the indentation

<groupId>com.huawei.paas.cse</groupId>
<artifactId>cse-adapter-springmvc</artifactId>
</dependency>
<groupId>com.netflix.ribbon</groupId>
Copy link
Member

Choose a reason for hiding this comment

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

please fix the indentation

* @author Sukesh
*/
@Configuration
@EnableConfigurationProperties
Copy link
Member

Choose a reason for hiding this comment

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

why EnableConfigurationProperties here?

String serviceName = config.getClientName();
String appID = cseRoutesProperties.getAppID();
String versionRule = cseRoutesProperties.getVersionRule(serviceName);
CseServerListWrapper serverList = new CseServerListWrapper(appID, serviceName, versionRule, "rest");
Copy link
Member

Choose a reason for hiding this comment

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

please inline serverList

@Autowired
private ZuulProperties zuulProperties;

private Map<String, String> appServiceMap = null;
Copy link
Member

Choose a reason for hiding this comment

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

appServiceMap shall be final

public ServiceRouteMapper cseServiceRouteMapper() {
return new CseServiceRouteMapper();
@Bean
@ConditionalOnMissingBean
Copy link
Member

Choose a reason for hiding this comment

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

why does ConditionalOnMissingBean have no bean class provided?

<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-starter-zuul</artifactId>
<version>1.3.0.RELEASE</version>
Copy link
Member

Choose a reason for hiding this comment

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

can we use a single spring cloud parent instead of managing all the versions ourselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

once the versions base lined, will optimize all the version configurations

<dependency>
<groupId>com.netflix.ribbon</groupId>
<artifactId>ribbon-loadbalancer</artifactId>
<version>2.2.2</version>
Copy link
Member

Choose a reason for hiding this comment

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

the versions can be managed in parent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

once the versions base lined, will optimize all the version configurations

<logback.version>1.1.7</logback.version>
<spring-cloud-commons.version>1.2.0.RELEASE</spring-cloud-commons.version>
<spring-cloud-netflix.version>1.3.0.RELEASE</spring-cloud-netflix.version>
<spring-cloud-commons.version>1.1.0.RELEASE</spring-cloud-commons.version>
Copy link
Member

Choose a reason for hiding this comment

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

we shall use a spring cloud parent to manage the dependencies, or we may have version conflicts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

once the versions base lined, will optimize all the version configurations


//TODO to load when webapplication context is used for discovery client, need to check if can use the order and undo this change with proper fix.
if(!isInit)
{
Copy link
Member

Choose a reason for hiding this comment

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

the code style is not correct

…l/EnableAutoDiscovery

Signed-off-by: sukesh a c <sukeshac@huawei.com>
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 305429e on acsukesh:master into ** on WillemJiang:master**.

…l/EnableAutoDiscovery && meregr from master

Signed-off-by: sukesh a c <sukeshac@huawei.com>
@seanyinx seanyinx merged commit 7b73305 into apache:master May 22, 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.

3 participants