jocko icon indicating copy to clipboard operation
jocko copied to clipboard

initial PR for simple admin client api and small admin client app

Open yglcode opened this issue 7 years ago • 3 comments

purpose:

  1. add simple admin api derived KIP117 and java code (https://github.com/confluentinc/kafka/tree/trunk/clients/src/main/java/org/apache/kafka/clients/admin)
  2. add client app, more consistent with Go cmdline (cobra) conventions, convenient for exploration of cluster resources, such as, kadm init -b kafka1:9092,kafka2:9092 kadm topic list kadm topic create -p 10 -r 3 topic1 topic2 topic3 kadm topic delete t1 t2 t3 kadm node list of course, can still target command at specific broker: kadm group list --brokers kafka1:9092

yglcode avatar Apr 25 '18 03:04 yglcode

Codecov Report

Merging #118 into master will increase coverage by 0.35%. The diff coverage is 9.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #118      +/-   ##
==========================================
+ Coverage    41.4%   41.76%   +0.35%     
==========================================
  Files          84       85       +1     
  Lines        5936     5572     -364     
==========================================
- Hits         2458     2327     -131     
+ Misses       3022     2801     -221     
+ Partials      456      444      -12
Impacted Files Coverage Δ
client/dialer.go 60% <ø> (ø)
client/state.go 0% <0%> (ø)
protocol/metadata_request.go 0% <0%> (ø) :arrow_up:
protocol/metadata_response.go 0% <0%> (ø) :arrow_up:
client/admin.go 0% <0%> (ø)
client/client.go 0% <0%> (ø)
jocko/broker.go 52.11% <100%> (-1.52%) :arrow_down:
jocko/leader.go 61% <100%> (+0.44%) :arrow_up:
client/conn.go 56.35% <61.53%> (ø)
... and 72 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 3497f59...50f7416. Read the comment docs.

codecov[bot] avatar Apr 25 '18 04:04 codecov[bot]

Good work homie. Here's some layout changes to make:

  1. Let's move the conn and dialer back into the jocko pkg. We can export those to use/wrap from the client pkg. The jocko pkg shouldn't have to import the client pkg.
  2. Move the admin commands under the jocko cmd so there's one binary. I think:
jocko broker
jocko topic create|delete|list|describe
jocko group list|describe
jocko partition list|reset|describe
jocko node list|describe|api-version

Look good, similar setup Hashicorp uses.

When you've got those changes in place we can work on some style changes.

Thanks.

travisjeffery avatar Apr 26 '18 09:04 travisjeffery

Thanks for your comments. Here are some more thoughts.

  1. Yes jocko/broker pkg shouldn’t need to import/pull-in client pkg which are most API handling. In fact jock/broker and client are quite independent, only sharing conn/dial code and protocol code. Protocol code has already been separated into its own pkg. We could move conn/dial code to a “conn” pkg, so that client and broker can build independently by importing “protocol” and “conn”, without pulling in other nonused code.

  2. I’ll add admin commands to jocko binary. It seems it is also good to keep a separate admin client binary, because jock broker code are more advanced and new features will take time to iron out. Client code is quite simple, mostly check KIPs and copy over Java code. Also can use admin client against apache Kafka servers and test if the protocol code is correct or not. If we keep them separate, then we can release them in diff schedule.

Thanks

yglcode avatar Apr 27 '18 02:04 yglcode