-
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
Fix service discovery related issues and add mesh proxy mode support #2022
Conversation
chickenlj
commented
Aug 18, 2022
•
edited
Loading
edited
- add 'mesh-enabled' property, 'true' means to deploy SDK together with sidecar
- make 'registry-type' to support 'service', 'interface', and 'all', representing register instance address, register interface address, and register both respectively. Default behavior is 'service'
- fix instance address notification issue
Samples would be added to dubbo-go-samples soon |
Codecov Report
@@ Coverage Diff @@
## 3.1 #2022 +/- ##
==========================================
- Coverage 44.60% 44.58% -0.03%
==========================================
Files 283 283
Lines 17105 17215 +110
==========================================
+ Hits 7630 7675 +45
- Misses 8669 8730 +61
- Partials 806 810 +4
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Merge this pull request first to pass the CI. |
please squash your commits with git rebase. |
Why is this necessary? I think 'squash and merge' would do the same thing for us if you worry too many commits would be brought in the repo after merging, it will compose all commits into one commit and the only thing that needs to do when using that is to replace the commit record listed there to one single conclusion. |
|
Kindly remind: please use 'Squash and Merge' when merging. |
ok = false | ||
} | ||
|
||
return r, ok |
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.
这个是不是多余了?直接判断是否为空字符串不就好了吗?
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.
return r, r == ""
@@ -515,6 +530,23 @@ func (c *URL) GetParam(s string, d string) string { | |||
return r | |||
} | |||
|
|||
// GetParamNoDefault gets value by key, return nil,false if no value found mapping to the key | |||
func (c *URL) GetParamNoDefault(s string) (string, bool) { |
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.
这个函数名气很奇怪。在Go里面,我们知道map之类的会有一些default值,所以我大概也能明白你的用意。但 (string, bool) 中的 bool 就是 go 的风格,能够说明 key 是否存在,所以改名叫 GetParam 就可以了。
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.
改名叫 GetParam 就可以
GetParam 这个名字已经被占用了,并且没有 bool 返回值,没啥好办法了。
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.
You might consider using GetParamWithoutDefault
to replace that.
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.
GetNonDefaultParam怎么样
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.
GetNonDefaultParam怎么样
Agree with u. Really a better func name.
GetNonDefaultParam |