You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2021/07/12 08:49:35 UTC
[GitHub] [dubbo-go] xavier-niu opened a new pull request #1315: Ftr: Generic invocation supports "protobuf-json"
xavier-niu opened a new pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315
<!-- Thanks for sending a pull request!
Read https://github.com/apache/dubbo-go/blob/master/CONTRIBUTING.md before commit pull request.
-->
**What this PR does**:
- Ftr: Generic invocation supports "protobuf-json"
**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #
**Special notes for your reviewer**:
**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
```
--
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
[GitHub] [dubbo-go] xavier-niu edited a comment on pull request #1315: Ftr: Generic invocation supports Generalizer
Posted by GitBox <gi...@apache.org>.
xavier-niu edited a comment on pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315#issuecomment-886061723
> Could you make github action to success?
因为改动了接口，原有的samples不能正常运行，目前我已经针对这个改进在samples中提了PR，但是需要等待该PR合并。我已经在本地模式运行过samples，逻辑上没有问题。
--
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
[GitHub] [dubbo-go] xavier-niu commented on pull request #1315: Ftr: Generic invocation supports Generalizer
Posted by GitBox <gi...@apache.org>.
xavier-niu commented on pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315#issuecomment-886061723
> Could you make github action to success?
Of course, please see apache/dubbo-go-samples#170.
--
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
[GitHub] [dubbo-go] xavier-niu edited a comment on pull request #1315: Ftr: Generic invocation supports Generalizer
Posted by GitBox <gi...@apache.org>.
xavier-niu edited a comment on pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315#issuecomment-886061723
> Could you make github action to success?
Of course, please see apache/dubbo-go-samples#170. The samples work fine in local test, so once this pr was merged, I will address the conflict for samples pr.
--
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
[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1315: Ftr: Generic invocation supports Generalizer
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315#discussion_r675988762
##########
File path: common/rpc_service.go
##########
@@ -366,7 +366,7 @@ func suiteMethod(method reflect.Method) *MethodType {
// The latest return type of the method must be error.
if returnType := mtype.Out(outNum - 1); returnType != typeOfError {
- logger.Warnf("the latest return type %s of method %q is not error", returnType, mname)
+ logger.Debugf(`"%s" method will not be exported because the return type not includes error`, mname, returnType)
Review comment:
"%s" method will not be exported because its return type %v do not have error
--
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
[GitHub] [dubbo-go] xavier-niu commented on pull request #1315: Ftr: Generic invocation supports "protobuf-json"
Posted by GitBox <gi...@apache.org>.
xavier-niu commented on pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315#issuecomment-878182691
> make action success pls.
This PR changes some behavior of generic invocation that makes the CI failed, I've opened a new PR to solve this, please see https://github.com/apache/dubbo-go-samples/pull/170.
--
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
[GitHub] [dubbo-go] LaurenceLiZhixin merged pull request #1315: Ftr: Generic invocation supports Generalizer
Posted by GitBox <gi...@apache.org>.
LaurenceLiZhixin merged pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315
--
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
[GitHub] [dubbo-go] LaurenceLiZhixin commented on a change in pull request #1315: Ftr: Generic invocation supports "protobuf-json"
Posted by GitBox <gi...@apache.org>.
LaurenceLiZhixin commented on a change in pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315#discussion_r671808185
##########
File path: filter/generic/generalizer/example.proto
##########
@@ -0,0 +1,35 @@
+/*
Review comment:
这个文件不用放在这里吧，框架代码中尽量不包含example内容。
--
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
[GitHub] [dubbo-go] xavier-niu edited a comment on pull request #1315: Ftr: Generic invocation supports "protobuf-json"
Posted by GitBox <gi...@apache.org>.
xavier-niu edited a comment on pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315#issuecomment-878182691
> make action success pls.
This PR changes some behavior of generic invocation that makes the CI failed, I've opened a new PR for dubbo-go-samples to make integrated tests pass, please see https://github.com/apache/dubbo-go-samples/pull/170.
--
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
[GitHub] [dubbo-go] LaurenceLiZhixin commented on pull request #1315: Ftr: Generic invocation supports Generalizer
Posted by GitBox <gi...@apache.org>.
LaurenceLiZhixin commented on pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315#issuecomment-885587136
Pls fix your conflict and push me to review, tks.
--
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
[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1315: Ftr: Generic invocation supports Generalizer
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315#discussion_r675989615
##########
File path: filter/generic/generalizer/protobuf_json.go
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package generalizer
+
+import (
+ "reflect"
+ "sync"
+)
+
+import (
+ perrors "github.com/pkg/errors"
+ "google.golang.org/protobuf/encoding/protojson"
+ "google.golang.org/protobuf/proto"
+)
+
+var (
+ protobufJsonGeneralizer Generalizer
+ protobufJsonGeneralizerOnce sync.Once
+)
+
+func GetProtobufJsonGeneralizer() Generalizer {
+ protobufJsonGeneralizerOnce.Do(func() {
+ protobufJsonGeneralizer = &ProtobufJsonGeneralizer{}
+ })
+ return protobufJsonGeneralizer
+}
+
+type ProtobufJsonGeneralizer struct{}
Review comment:
pls add comment for this struct and its methods.
--
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
[GitHub] [dubbo-go] xavier-niu commented on pull request #1315: Ftr: Generic invocation supports Generalizer
Posted by GitBox <gi...@apache.org>.
xavier-niu commented on pull request #1315:
URL: https://github.com/apache/dubbo-go/pull/1315#issuecomment-885742671
> Pls fix your conflict and push me to review, tks.
Done, now this PR is ready for review.
--
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