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

Ftr: consul registry #121

Merged
merged 54 commits into from
Aug 26, 2019
Merged

Ftr: consul registry #121

merged 54 commits into from
Aug 26, 2019

Conversation

gaoxinge
Copy link

@gaoxinge gaoxinge commented Jul 4, 2019

What this PR does:

Add consul registry.

Special notes for your reviewer:

First thing that should be noticed is that this pr only support golang 1.12, because native consul api only support 1.12:

Another thing is that there are still a lot of things should be done about consul registry:

  • add unit test
  • change the gradle to maven in consul examples
  • add more docs about examples
  • support data center/config center

@hxmhlt @fangyincheng @AlexStocks

@hxmhlt
Copy link
Contributor

hxmhlt commented Jul 5, 2019

Pls add unit test and resolve the conflicts.

@gaoxinge
Copy link
Author

gaoxinge commented Jul 5, 2019

@hxmhlt Ok.

registry/consul/registry.go Outdated Show resolved Hide resolved
registry/consul/listener.go Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Jul 9, 2019

Codecov Report

Merging #121 into develop will increase coverage by 0.3%.
The diff coverage is 71.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop     #121     +/-   ##
==========================================
+ Coverage    67.54%   67.84%   +0.3%     
==========================================
  Files           90       93      +3     
  Lines         5537     5729    +192     
==========================================
+ Hits          3740     3887    +147     
- Misses        1403     1432     +29     
- Partials       394      410     +16
Impacted Files Coverage Δ
registry/directory/directory.go 78.3% <ø> (ø) ⬆️
common/url.go 70.68% <ø> (ø) ⬆️
remoting/zookeeper/client.go 63.58% <0%> (ø) ⬆️
config/config_loader.go 53.73% <0%> (ø) ⬆️
registry/nacos/registry.go 74% <100%> (ø) ⬆️
remoting/zookeeper/listener.go 46.9% <28.57%> (ø) ⬆️
registry/consul/registry.go 69.49% <69.49%> (ø)
registry/consul/utils.go 71.11% <71.11%> (ø)
registry/consul/listener.go 81.48% <81.48%> (ø)
config_center/zookeeper/listener.go 82.6% <0%> (-4.35%) ⬇️
... and 4 more

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 00a2553...5dd2bd0. Read the comment docs.

registry/consul/listener.go Outdated Show resolved Hide resolved
registry/consul/listener.go Outdated Show resolved Hide resolved
registry/consul/listener.go Show resolved Hide resolved
registry/consul/registry.go Outdated Show resolved Hide resolved
registry/consul/utils.go Outdated Show resolved Hide resolved
registry/consul/utils.go Outdated Show resolved Hide resolved
examples/consul/go-server/server.go Show resolved Hide resolved
return err
}
}
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Why only register provider side? Where is consumer's registry?

Copy link
Author

@gaoxinge gaoxinge Aug 23, 2019

Choose a reason for hiding this comment

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

@AlexStocks AlexStocks changed the title consul Ftr: consul registry Aug 23, 2019
examples/consul/go-server/config/server.yml Outdated Show resolved Hide resolved
registry/consul/listener.go Show resolved Hide resolved
registry/consul/listener.go Outdated Show resolved Hide resolved
func (r *consulRegistry) Unregister(url common.URL) error {
var err error

role, _ := strconv.Atoi(r.URL.GetParam(constant.ROLE_KEY, ""))
Copy link
Contributor

Choose a reason for hiding this comment

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

need not to unregister consumer?

Copy link
Author

Choose a reason for hiding this comment

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

registry/consul/listener.go Show resolved Hide resolved
@hxmhlt hxmhlt merged commit 9a7642b into apache:develop Aug 26, 2019
@gaoxinge gaoxinge deleted the consul branch August 26, 2019 04:47
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.

5 participants