-
Notifications
You must be signed in to change notification settings - Fork 85
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
dist: drop Java tools and change packaging just for cassandra-stress #384
base: master
Are you sure you want to change the base?
dist: drop Java tools and change packaging just for cassandra-stress #384
Conversation
Architecture: all | ||
Depends: %{product}-conf, %{product}-tools-core, ${misc:Depends} | ||
Priority: optional |
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.
since you are specifying the "Priority" of this package. i'd suggest either do the same to ${product}-cassandra-stress
, or just drop both of them. because, according to Debian Policy Manual,
extra
This priority is deprecated. Use the optional priority instead. This priority should be treated as equivalent to optional.
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.
Added Priority: optional
to %{product}-cassandra-stress and %{product}-cassandra-stress.
|
||
Package: %{product}-cassandra-stress | ||
Architecture: all | ||
Depends: %{product}-conf, %{product}-cassandra-stress-core, ${misc:Depends} | ||
Recommends: ntp | time-daemon |
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.
ntp
and time-daemon
were added in https://issues.apache.org/jira/browse/CASSANDRA-4606 and https://issues.apache.org/jira/browse/CASSANDRA-5816 respectively. they intended to run a cassandra cluster . but cassandra-stress as a client-side tool, it does not rely on a ntp or chrony service to operate. so i'd suggest take this opportunity to drop them.
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.
Dropped.
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.
lgtm. just a couple nits. but they are not blockers.
I think scylladb/scylla-ccm#565 has to go in, before we can merge this. |
448006b
to
e82d976
Compare
@denesb It was merged yesterday, can we merge this one? |
Not yet. The above PR was reduced in scope to just prepare ccm for the post JMX world. Throwing out all of Java on on go proved too big of a bite, to chew at once. I just realized we also didn't yet finish scylladb/scylladb#14856, notably, there are still dtests around testing the java tools. These need to be either:
|
e82d976
to
ee4654d
Compare
Rebased, since it was conflicted. |
I need to modify Obsoletes/Breaks/Replaces on this patch, since I supposed next release version is 5.5 so I wrote something like |
Since Java based tools are deprecated and now we only need cassandra-stress, change package name to "scylla-cassandra-stress" and drop deprecated tools from the package. Closes scylladb#370
ee4654d
to
fbf55a3
Compare
Replaced |
I think this one is not needed and this task #370 misled you. We can/should keep the packaging as "scylla-tools-java" and c-s will be part of it. So, the packaging can remain as scylla-tools-java but just not a dependency of scylladb. |
@syuu1228 I guess we can continue with that now, right? |
Since Java based tools are deprecated and now we only need cassandra-stress, change package name to "scylla-cassandra-stress" and drop deprecated tools from the package.
Closes #370