Skip to content
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(contrib): get nacos service of customize group name #1798

Merged
merged 13 commits into from
Feb 22, 2022

Conversation

JinPengCN
Copy link
Contributor

@JinPengCN JinPengCN commented Jan 28, 2022

Description (what this PR does / why we need it):

registry.GetService调用了nacos-sdk-go包中的SelectInstances方法:
···
func (sc *NamingClient) SelectInstances(param vo.SelectInstancesParam) ([]model.Instance, error) {
if len(param.GroupName) == 0 {
param.GroupName = constant.DEFAULT_GROUP
}
.......
}
···
如果没有指定param.GroupName,则取默认的分组名称。目前kratos的实现中,registry.GetService方法中只设置了ServiceName,所以无法获取自定义分组名称的服务。为了支持这种情况,增加GroupName参数的设置。

Which issue(s) this PR fixes (resolves / be part of):

fixes #1801

Other special notes for reviewer:

registry.GetService调用了nacos-sdk-go包中的SelectInstances方法:
···
func (sc *NamingClient) SelectInstances(param vo.SelectInstancesParam) ([]model.Instance, error) {
	if len(param.GroupName) == 0 {
		param.GroupName = constant.DEFAULT_GROUP
	}
        .......
}
···
如果没有指定param.GroupName,则取默认的分组名称。目前kratos的实现中,registry.GetService接口参数签名,只支持serviceName,所以无法获取自定义分组名称的服务。为了支持这种情况,加了字符串分隔。跟官方保持一致,分隔符采用“@@”.
修改之后,业务中这样调用:
//GroupName为自定义的分组名称
registry.GetService(ctx, "GroupName@@ServiceName")  
//默认的分组名称
registry.GetService(ctx, "ServiceName")
@JinPengCN JinPengCN changed the title 获取nacos注册的服务,支持自定义分组 fix(angular): get nacos service of customize group name Jan 28, 2022
@JinPengCN JinPengCN changed the title fix(angular): get nacos service of customize group name fix(registry): get nacos service of customize group name Jan 28, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 28, 2022

Codecov Report

Merging #1798 (0b0a62a) into main (86b8b6c) will not change coverage.
The diff coverage is n/a.

❗ Current head 0b0a62a differs from pull request most recent head 3b3ac52. Consider uploading reports for the commit 3b3ac52 to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1798   +/-   ##
=======================================
  Coverage   80.80%   80.80%           
=======================================
  Files          80       80           
  Lines        3558     3558           
=======================================
  Hits         2875     2875           
  Misses        477      477           
  Partials      206      206           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86b8b6c...3b3ac52. Read the comment docs.

@JinPengCN JinPengCN changed the title fix(registry): get nacos service of customize group name fix(registry/nacos): get nacos service of customize group name Jan 28, 2022
@JinPengCN JinPengCN closed this Jan 28, 2022
@JinPengCN JinPengCN changed the title fix(registry/nacos): get nacos service of customize group name fix(contrib): get nacos service of customize group name Jan 28, 2022
@JinPengCN JinPengCN reopened this Jan 28, 2022
Copy link
Contributor Author

@JinPengCN JinPengCN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove judge

Nacos service need the custom group support

fix(crntrib): add the splitServiceNameNum constant

deps:upgrade go mod version (go-kratos#1800)

* deps:upgrade go mod version

fix: allocates new objects each time (go-kratos#1802)

fix: fix ci tool (go-kratos#1803)

* fix: fix ci tool

fix(crntrib): add the splitServiceNameSep constant

fix(crntrib): get GroupName from opts

fix(crntrib): get GroupName from opts

fix(crntrib): get GroupName from opts

Update registry.go
@tonybase tonybase merged commit 85800ce into go-kratos:main Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The nacos implementation of discovery interface ,custom group name is not supported
3 participants