-
Notifications
You must be signed in to change notification settings - Fork 929
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ftr: add pass through proxy factory #1081
Conversation
common/constant/key.go
Outdated
@@ -50,7 +50,8 @@ const ( | |||
PROTOCOL_KEY = "protocol" | |||
PATH_SEPARATOR = "/" | |||
//DUBBO_KEY = "dubbo" | |||
SSL_ENABLED_KEY = "ssl-enabled" | |||
SSL_ENABLED_KEY = "ssl-enabled" | |||
ParameterTypeKey = "parameter-type-names" // ParameterType key used in dapr, to tranfer rsp param type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe follow the codeSSL_ENABLED_KEY
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
var args [][]byte | ||
if len(arguments) == 0 { | ||
args = [][]byte{} | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can reduce a if-else
branch by
if len(arguments) > 0 {
args = make([][]byte, 0, len(arguments))
for _, arg := range arguments {
if v, ok := arg.([]byte); ok {
args = append(args, v)
} else {
result.Err = perrors.New("the param type is not []byte")
return result
}
}
}
} | ||
method := srv.Method()["Service"] | ||
|
||
in := []reflect.Value{srv.Rcvr()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Init with length in := make([]reflect.Value, 5)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
replyv := returnValues[0] | ||
retErr = returnValues[1].Interface() | ||
|
||
if retErr != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe more appropriate.
if retErr != nil {
result.SetError(retErr.(error))
return result
}
if replyv.IsValid() && (replyv.Kind() != reflect.Ptr || replyv.Kind() == reflect.Ptr && replyv.Elem().IsValid()) {
result.SetResult(replyv.Interface())
}
return result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed!
registry/directory/directory.go
Outdated
logger.Infof("selector add service url{%s}", event.Service) | ||
// FIXME: routers are built in every address notification? | ||
dir.configRouters() | ||
if event.Service != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if event != nil && event.Service != nil
README.md
Outdated
@@ -177,7 +177,7 @@ If you are willing to do some code contributions and document contributions to [ | |||
|
|||
## Community | |||
|
|||
If u want to communicate with our community, pls scan the following [dubbobo Ding-Ding QR code](https://mmbiz.qpic.cn/mmbiz_jpg/yvBJb5IiafvnHVBdtia30dxA2hKotr9DEckWsZ7aOJcDWDaSVMGwLmYv8GRgIQtqb4C2svicp8nVkMmGy7yKC5tyA/640?wx_fmt=jpeg&tp=webp&wxfrom=5&wx_lazy=1&wx_co=1) or search our commnity DingDing group code 31363295. | |||
If u want to communicate with our community, pls scan the following dubbobo Ding-Ding QR code or search our commnity DingDing group code 31363295. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Ding-Ding " -> DingDing
|
||
var args [][]byte | ||
if len(arguments) > 0 { | ||
args = make([][]byte, 0, len(arguments)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same comment as line 93: pls using gxbytes.AcquireBytes instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gxbytes.AcquireBytes() is used to get specific make([]byte, 0, size), not to make [][]byte.
Codecov Report
@@ Coverage Diff @@
## 3.0 #1081 +/- ##
==========================================
- Coverage 60.03% 59.36% -0.68%
==========================================
Files 260 269 +9
Lines 12855 13441 +586
==========================================
+ Hits 7718 7979 +261
- Misses 4174 4475 +301
- Partials 963 987 +24
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved these changes
Ftr: add pass through proxy factory
What this PR does:
add pass through factory
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
pass through proxy invoker can invoke user defined service, and pass through all data to user defined service. User can wirte logic to deal with raw invocation data.
It is very useful to dapr-dubbo support.
Does this PR introduce a user-facing change?: