You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "chiacyu (via GitHub)" <gi...@apache.org> on 2024/12/01 01:29:30 UTC
[PR] KAFKA-17836: Move RackAwareTest to server module [kafka]
chiacyu opened a new pull request, #17998:
URL: https://github.com/apache/kafka/pull/17998
`RackAwareTest` and `AdminRackAwareTest` should be moved to server's org.apache.kafka.admin package. Also, `RackAwareAutoTopicCreationTest` can be removed as it is for zk only.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17836: Move RackAwareTest to server module [kafka]
Posted by "chiacyu (via GitHub)" <gi...@apache.org>.
chiacyu commented on PR #17998:
URL: https://github.com/apache/kafka/pull/17998#issuecomment-2514613473
Hi, @chia7712
Please take a look while you're available, thanks!.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17836: Move RackAwareTest to server module [kafka]
Posted by "chiacyu (via GitHub)" <gi...@apache.org>.
chiacyu commented on PR #17998:
URL: https://github.com/apache/kafka/pull/17998#issuecomment-2509514283
The original discussion is https://github.com/apache/kafka/pull/17646, this new one is to solve the merge conflicts. @frankvicky , please take a look while you're available.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17836: Move RackAwareTest to server module [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on code in PR #17998:
URL: https://github.com/apache/kafka/pull/17998#discussion_r1864732418
##########
server-common/src/main/java/org/apache/kafka/admin/ReplicaDistributions.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.admin;
+
+import java.util.List;
+import java.util.Map;
+
+public class ReplicaDistributions {
Review Comment:
This class is only for test so I think we don't need to put in `src`.
Maybe it can be inner class in `RackAwareTest`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17836: Move RackAwareTest to server module [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 closed pull request #17998: KAFKA-17836: Move RackAwareTest to server module
URL: https://github.com/apache/kafka/pull/17998
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17836: Move RackAwareTest to server module [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17998:
URL: https://github.com/apache/kafka/pull/17998#issuecomment-2716336355
This PR is being marked as stale since it has not had any activity in 90 days. If you
would like to keep this PR alive, please leave a comment asking for a review. If the PR has
merge conflicts, update it with the latest from the base branch.
<p>
If you are having difficulty finding a reviewer, please reach out on the
[mailing list](https://kafka.apache.org/contact).
<p>
If this PR is no longer valid or desired, please feel free to close it. If no activity
occurs in the next 30 days, it will be automatically closed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17836: Move RackAwareTest to server module [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #17998:
URL: https://github.com/apache/kafka/pull/17998#issuecomment-2535339509
> Are RackAwareTest and AdminRackAwareTest testing any logic used when not running with ZooKeeper? It seems all the callers of the methods they check are from ZooKeeper logic.
yes, `AdminRackAwareTest` can be removed directly, but `RackAwareAutoTopicCreationTest` is running under kraft now. Maybe we can remove `AdminRackAwareTest` and then merge `RackAwareTest` and `RackAwareAutoTopicCreationTest` into single class?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17836: Move RackAwareTest to server module [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on PR #17998:
URL: https://github.com/apache/kafka/pull/17998#issuecomment-2531311818
Are `RackAwareTest` and `AdminRackAwareTest` testing any logic used when not running with ZooKeeper? It seems all the callers of the methods they check are from ZooKeeper logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17836: Move RackAwareTest to server module [kafka]
Posted by "chiacyu (via GitHub)" <gi...@apache.org>.
chiacyu commented on PR #17998:
URL: https://github.com/apache/kafka/pull/17998#issuecomment-2532075892
Hi, @mimaison
I've check the logic and if I'm not missing anything, these methods are used by the service related Zookeeper.
Should we just remove all since we're removing Zookeeper dependency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17836: Move RackAwareTest to server module [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on PR #17998:
URL: https://github.com/apache/kafka/pull/17998#issuecomment-2532104441
Yes we should not spend time rewritting them as we will be deleting them soon.
However I don't think we're ready to delete them yet, see https://issues.apache.org/jira/browse/KAFKA-17611 for the progress on the ZooKeeper deletion.
So let's close this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org