Skip to content
This repository was archived by the owner on Mar 31, 2023. It is now read-only.

Conversation

@kevin-zhonghao
Copy link
Contributor

@kevin-zhonghao kevin-zhonghao commented May 15, 2020

This PR proposes the following changes:

  • Refactor entities reference structure. Now all micro-services use entities from the AlcorWeb.
  • Add verify request resource module and configure properties default value module for vpc, segment and subnet.

Copy link
Contributor

@xieus xieus left a comment

Choose a reason for hiding this comment

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

Some initial feedback. Will keep looking.


import com.futurewei.alcor.web.entity.ip.IpAddrRequest;
import com.futurewei.alcor.web.restclient.IpManagerRestClient;
import com.futurewei.alcor.portmanager.restclient.IpManagerRestClient;
Copy link
Contributor

Choose a reason for hiding this comment

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

oh why do we want to move rest clients from AlcroWeb back to Port Manager?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Piaoping make the change I think ~

#apache.kafka.address=172.17.0.1:9092
#server.port=9000
#Ignite configuration
#ignite.host=192.168.137.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Switching back and forward on different host ip is kind of headache. Let us create a new set of application.properties to be more developer friendly :-) when you get a chance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

Copy link
Contributor

@xieus xieus left a comment

Choose a reason for hiding this comment

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

A few more minor comments. I think this PR is good shape. Let us address the comments and merge it soon.

return true;
}

public static String localToUTC(String localTime, String format) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering if we could get a UTC time directly, instead of time zone conversion.

@xieus xieus added the enhancement New feature or request label May 21, 2020
@xieus xieus added this to the Version 0.5.2020.05.30 milestone May 21, 2020
@xieus xieus changed the title [Micro-service] Fix issues in swagger test [Micro service] Consolidate all web-related entities under AlcorWeb, add validation to VPC/Sunet Manager May 21, 2020
@xieus xieus changed the title [Micro service] Consolidate all web-related entities under AlcorWeb, add validation to VPC/Sunet Manager [Micro service] Consolidate web-related entities under AlcorWeb, add validation to VPC/Sunet Manager May 21, 2020
# Conflicts:
#	services/mac_manager/pom.xml
#	services/mac_manager/src/main/java/com/futurewei/alcor/macmanager/controller/MacController.java
#	services/mac_manager/src/main/java/com/futurewei/alcor/macmanager/dao/MacPoolRepository.java
#	services/mac_manager/src/main/java/com/futurewei/alcor/macmanager/dao/MacRangeRepository.java
#	services/mac_manager/src/main/java/com/futurewei/alcor/macmanager/service/MacService.java
#	services/mac_manager/src/main/java/com/futurewei/alcor/macmanager/service/implement/MacServiceImpl.java
#	services/mac_manager/src/main/java/com/futurewei/alcor/macmanager/utils/RestPreconditionsUtil.java
#	services/mac_manager/src/main/resources/application.properties
#	services/mac_manager/src/test/java/com/futurewei/alcor/macmanager/controller/MacControllerTest.java
#	services/node_manager/src/main/resources/application.properties
#	web/src/main/java/com/futurewei/alcor/web/entity/mac/MacPool.java
# Conflicts:
#	services/mac_manager/src/main/resources/application.properties
Copy link
Contributor

@xieus xieus left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for consolidating all the entities into one place, which take a lot of efforts but save us time when comes to integration :-)

@xieus xieus merged commit 822f4f5 into futurewei-cloud:master May 22, 2020
@kevin-zhonghao kevin-zhonghao deleted the master branch May 22, 2020 19:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Micro-service] Subnet Manager [Micro-service] Route manager [Micro-service] VPC manager

2 participants