You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "DMwangnima (via GitHub)" <gi...@apache.org> on 2023/12/07 09:05:26 UTC

[PR] feat: support triple group/version division for a certain Interface [dubbo-go]

DMwangnima opened a new pull request, #2532:
URL: https://github.com/apache/dubbo-go/pull/2532

   Now, Triple could support dividing Group/Version for a same Interface.
   Please see /protocol/triple/triple_test.go for detail.


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

Re: [PR] feat: support triple group/version division for a certain Interface [dubbo-go]

Posted by "DMwangnima (via GitHub)" <gi...@apache.org>.
DMwangnima merged PR #2532:
URL: https://github.com/apache/dubbo-go/pull/2532


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

Re: [PR] feat: support triple group/version division for a certain Interface [dubbo-go]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #2532:
URL: https://github.com/apache/dubbo-go/pull/2532#issuecomment-1844956563

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=2532)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2532&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2532) No Coverage information  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2532&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2532&metric=new_duplicated_lines_density&view=list)
   
   


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

Re: [PR] feat: support triple group/version division for a certain Interface [dubbo-go]

Posted by "FinalT (via GitHub)" <gi...@apache.org>.
FinalT commented on code in PR #2532:
URL: https://github.com/apache/dubbo-go/pull/2532#discussion_r1419080486


##########
protocol/triple/triple_protocol/handler.go:
##########
@@ -255,7 +341,11 @@ func (h *Handler) ServeHTTP(responseWriter http.ResponseWriter, request *http.Re
 		return
 	}
 	// invoke implementation
-	_ = connCloser.Close(h.implementation(ctx, connCloser))
+	svcGroup := request.Header.Get(tripleServiceGroup)
+	svcVersion := request.Header.Get(tripleServiceVersion)
+	implementation := h.implementations[getIdentifier(svcGroup, svcVersion)]

Review Comment:
   We need to check if implementation exists



##########
protocol/triple/triple_protocol/handler.go:
##########
@@ -255,7 +341,11 @@ func (h *Handler) ServeHTTP(responseWriter http.ResponseWriter, request *http.Re
 		return
 	}
 	// invoke implementation
-	_ = connCloser.Close(h.implementation(ctx, connCloser))
+	svcGroup := request.Header.Get(tripleServiceGroup)
+	svcVersion := request.Header.Get(tripleServiceVersion)
+	implementation := h.implementations[getIdentifier(svcGroup, svcVersion)]

Review Comment:
   If this implementation doesn't exist, the system will panic and the client won't get any response



##########
protocol/triple/triple_protocol/handler.go:
##########
@@ -255,7 +341,11 @@ func (h *Handler) ServeHTTP(responseWriter http.ResponseWriter, request *http.Re
 		return
 	}
 	// invoke implementation
-	_ = connCloser.Close(h.implementation(ctx, connCloser))
+	svcGroup := request.Header.Get(tripleServiceGroup)
+	svcVersion := request.Header.Get(tripleServiceVersion)
+	implementation := h.implementations[getIdentifier(svcGroup, svcVersion)]

Review Comment:
   Here we need to disconnect in time



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

Re: [PR] feat: support triple group/version division for a certain Interface [dubbo-go]

Posted by "DMwangnima (via GitHub)" <gi...@apache.org>.
DMwangnima commented on code in PR #2532:
URL: https://github.com/apache/dubbo-go/pull/2532#discussion_r1419203690


##########
protocol/triple/triple_protocol/handler.go:
##########
@@ -255,7 +341,11 @@ func (h *Handler) ServeHTTP(responseWriter http.ResponseWriter, request *http.Re
 		return
 	}
 	// invoke implementation
-	_ = connCloser.Close(h.implementation(ctx, connCloser))
+	svcGroup := request.Header.Get(tripleServiceGroup)
+	svcVersion := request.Header.Get(tripleServiceVersion)
+	implementation := h.implementations[getIdentifier(svcGroup, svcVersion)]

Review Comment:
   I need to check the Triple protocol specification in Dubbo since this is not an official feature in gRPC.
   That is the reason why I left a todo here.



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