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
[tools] package bookkeeper tools into a separated distribution #1798
Conversation
*Motivation* Make the tools available without pulling in server dependencies
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.
Good Idea. It will be useful
Does the script which checks licenses cover this new package?
- lib/org.apache.commons-commons-collections4-4.1.jar [18] | ||
- lib/org.apache.commons-commons-lang3-3.6.jar [19] | ||
- lib/org.apache.zookeeper-zookeeper-3.4.13.jar [20] | ||
- lib/org.rocksdb-rocksdbjni-5.13.1.jar [22] |
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 could exclude this as it's only relevant to bookies
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.
good point; excluded rocksdb
travis checks this package. |
run pr validation |
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.
👍
Hey @sijie whats the purpose of this change? what are you moving and how would it help? |
@reddycharan I am not moving anything. I am creating a new binary distribution that only includes tools. People can just download the tools package and use it to operate bookkeeper clusters without downloading the server package. |
@@ -0,0 +1,116 @@ | |||
<!-- |
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.
other files in this folder has 'bin' prefix,
bin-all.xml
bin-server.xml
may be you want to follow the same approach
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.
bin-all
and bin-server
are two server distributions for "server binary" distributions. so that's why it was prefixed with bin
(in the context, there was only one server binary distribution, the file was called bin
).
calling bkctl
is to differentiate the package from others.
@sijie i feel word 'tools' is kind of misleading. It kind of gives impression that it just contains tools to operate / administer bookkeeper like bookkeeper, bookkeeper-cli, bookkeeper http server stuff..reconsider the name for this package. And add more info. to this commit for what is the purpose for this package and how it differs from all/server package . when you say "People can just download the tools package and use it to operate bookkeeper clusters without downloading the server package.", one would ask what is wrong in using server package? |
@reddycharan updated the description. And if you have reviewed the PR, the package is not called ‘tools’, it is ‘bkctl’. |
@sijie thanks for explanation. Yes, I was little confused about purpose of this package. When you say "At many environments, you want to deliver a set of tools to your customers to interact with or operate bookkeeper clusters." do you mean to say this would be for customers who wants to download this package to their control nodes (not bookie nodes) and run bookkeeper shell script? so they would be able to run only commands in 'cluster'/'ledger' group but not in 'bookie' (commands which access local filesystem of bookie) group? |
yes. ideally the idea to have |
ok. lgtm. |
Descriptions of the changes in this PR:
Motivation
Server distribution contains a lot of stuffs that doesn’t need to run bkctl and bookkeeper shell, such as stats providers, http servers and many other dependencies.
At many environments, you want to deliver a set of tools to your customers to interact with or operate bookkeeper clusters. They don’t care about server side stuffs and ask them to use a server package to run a tool makes things confused.
changes
Create a new ‘bkctl’ package that only contains bookkeeper shell and new cli.