[GitHub] [dubbo-go-pixiu] ztelur commented on a diff in pull request #417: Lds basic

[GitBox <gi...@apache.org>]

ztelur commented on code in PR #417:
URL: https://github.com/apache/dubbo-go-pixiu/pull/417#discussion_r867448501


##########
pkg/config/xds/lds.go:
##########
@@ -102,42 +103,68 @@ func (l *LdsManager) removeListeners(toRemoveHash map[string]struct{}) {
 	names := make([]string, 0, len(toRemoveHash))
 	for name := range toRemoveHash {
 		names = append(names, name)
-		l.listenerMg.RemoveListener(names)
 	}
+	l.listenerMg.RemoveListener(names)
 }
 
 // setupListeners setup listeners accord to dynamic resource
 func (l *LdsManager) setupListeners(listeners []*xdsModel.Listener) {
+	//Make sure each one has a unique name like "host-port-protocol"
+	for _, v := range listeners {
+		v.Name = resolveListenerName(v.Address.SocketAddress.Address, int(v.Address.SocketAddress.Port), v.Protocol.String())

Review Comment:
   is it possible that v.Name  has it's value. check it is nil then call resolveListenerName ?



##########
pkg/config/xds/lds_test.go:
##########
@@ -138,3 +145,138 @@ http_filters:
 		})
 	}
 }
+
+func TestMakeListener(t *testing.T) {

Review Comment:
   well done for unit test



##########
samples/grpc/pixiu/conf.yaml:
##########
@@ -47,7 +47,6 @@ static_resources:
         - socket_address:
             address: 127.0.0.1
             port: 50001
-            protocol_type: "GRPC"

Review Comment:
   这个是有意修改嘛，感觉不会影响到这里



##########
pkg/listener/http/http_listener.go:
##########
@@ -80,6 +80,22 @@ func (ls *HttpListenerService) Start() error {
 	return nil
 }
 
+func (ls *HttpListenerService) Close() error {
+	return ls.srv.Close()
+}
+
+func (ls *HttpListenerService) ShutDown() error {
+	//TODO implement me
+	panic("implement me")

Review Comment:
   just remove panic, leave it empty implement? there are many cases



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org