You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/06/02 04:54:14 UTC
[GitHub] [kafka] ryannedolan opened a new pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
ryannedolan opened a new pull request #10805:
URL: https://github.com/apache/kafka/pull/10805
Per KIP-720, add deprecation warnings to legacy mirror maker.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r645317670
##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -58,7 +58,10 @@ import scala.util.{Failure, Success, Try}
* enable.auto.commit=false
* 3. Mirror Maker Setting:
* abort.on.send.failure=true
+ *
+ * @deprecated Since 3.0, use the Connect-based MirrorMaker instead (aka MM2).
*/
+@Deprecated
Review comment:
Since this is a Scala class, you should use the scala deprecated annotation.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mimaison merged pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
mimaison merged pull request #10805:
URL: https://github.com/apache/kafka/pull/10805
--
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
[GitHub] [kafka] showuon commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r643754484
##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -58,7 +58,11 @@ import scala.util.{Failure, Success, Try}
* enable.auto.commit=false
* 3. Mirror Maker Setting:
* abort.on.send.failure=true
+ *
+ * @deprecated The original Mirror Maker is deprecated since release 3.0. Similar functionality can be
+ * found in the Connect-based re-implementation by the same name (aka MM2).
Review comment:
1. `@deprecated The original Mirror Maker is deprecated since release 3.0.` -> could we just say `@deprecated since release 3.0.` ?
2. `Similar functionality can be found in the Connect-based re-implementation by the same name (aka MM2)` -> Could we change to: `Please use Connect-based re-implementation in org.apache.kafka.connect.mirror.MirrorMaker (aka MM2) instead.`?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ryannedolan commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
ryannedolan commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r644125908
##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -58,7 +58,11 @@ import scala.util.{Failure, Success, Try}
* enable.auto.commit=false
* 3. Mirror Maker Setting:
* abort.on.send.failure=true
+ *
+ * @deprecated The original Mirror Maker is deprecated since release 3.0. Similar functionality can be
+ * found in the Connect-based re-implementation by the same name (aka MM2).
Review comment:
Looking at other uses of the @deprecated javadoc tags, I see comments like "Since 2.4, use the PartitionReassignment Kafka API instead", which isn't far off from your suggestion. I'll reword, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ryannedolan commented on pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
ryannedolan commented on pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#issuecomment-873488063
I added a short note to the changelog.
--
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
[GitHub] [kafka] ryannedolan commented on pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
ryannedolan commented on pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#issuecomment-872203006
@ijuma can we merge this?
--
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
[GitHub] [kafka] ijuma commented on pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#issuecomment-873163159
@mimaison were you planning to merge this?
--
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
[GitHub] [kafka] mimaison commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
mimaison commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r663501513
##########
File path: docs/upgrade.html
##########
@@ -97,6 +97,9 @@ <h5><a id="upgrade_300_notable" href="#upgrade_300_notable">Notable changes in 3
<code>DefaultReplicationPolicy</code>, cannot prevent replication cycles based on topic names, so take care to avoid cycles when constructing your
replication topology.
</li>
+ <li> The original MirrorMaker (MM1) and related classes have been deprecated. Please use the Connect-based
+ MirrorMaker (MM2), as described in <a href="/{{version}}/documentation/#georeplication">.
Review comment:
`<a href="/{{version}}/documentation/#georeplication">` should be `the <a href="/{{version}}/documentation/#georeplication">Geo-Replication section</a>`
--
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
[GitHub] [kafka] ryannedolan commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
ryannedolan commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r663509639
##########
File path: docs/upgrade.html
##########
@@ -97,6 +97,9 @@ <h5><a id="upgrade_300_notable" href="#upgrade_300_notable">Notable changes in 3
<code>DefaultReplicationPolicy</code>, cannot prevent replication cycles based on topic names, so take care to avoid cycles when constructing your
replication topology.
</li>
+ <li> The original MirrorMaker (MM1) and related classes have been deprecated. Please use the Connect-based
+ MirrorMaker (MM2), as described in <a href="/{{version}}/documentation/#georeplication">.
Review comment:
fixed, thx
--
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
[GitHub] [kafka] showuon commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r643754484
##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -58,7 +58,11 @@ import scala.util.{Failure, Success, Try}
* enable.auto.commit=false
* 3. Mirror Maker Setting:
* abort.on.send.failure=true
+ *
+ * @deprecated The original Mirror Maker is deprecated since release 3.0. Similar functionality can be
+ * found in the Connect-based re-implementation by the same name (aka MM2).
Review comment:
1. `The original Mirror Maker` -> could we say `The old Mirror Maker` ?
2. `Similar functionality can be found in the Connect-based re-implementation by the same name (aka MM2)` -> Could we change to: `Please use Connect-based re-implementation in org.apache.kafka.connect.mirror.MirrorMaker (aka MM2) instead.`?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r645317670
##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -58,7 +58,10 @@ import scala.util.{Failure, Success, Try}
* enable.auto.commit=false
* 3. Mirror Maker Setting:
* abort.on.send.failure=true
+ *
+ * @deprecated Since 3.0, use the Connect-based MirrorMaker instead (aka MM2).
*/
+@Deprecated
Review comment:
Since this is a Scala class, you should use the scala deprecated annotation.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ryannedolan commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
ryannedolan commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r645646248
##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -58,7 +58,10 @@ import scala.util.{Failure, Success, Try}
* enable.auto.commit=false
* 3. Mirror Maker Setting:
* abort.on.send.failure=true
+ *
+ * @deprecated Since 3.0, use the Connect-based MirrorMaker instead (aka MM2).
*/
+@Deprecated
Review comment:
ah, I found a mix in use, but happy to switch.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mimaison commented on pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
mimaison commented on pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#issuecomment-858766776
This KIP was adopted on the basis of having an IdentityReplicationPolicy which is in this PR: https://github.com/apache/kafka/pull/10652
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ryannedolan commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
ryannedolan commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r663509197
##########
File path: docs/upgrade.html
##########
@@ -97,6 +97,9 @@ <h5><a id="upgrade_300_notable" href="#upgrade_300_notable">Notable changes in 3
<code>DefaultReplicationPolicy</code>, cannot prevent replication cycles based on topic names, so take care to avoid cycles when constructing your
replication topology.
</li>
+ <li> The original MirrorMaker (MM1) and related classes have been deprecated. Please use the Connect-based
+ MirrorMaker (MM2), as described in <a href="/{{version}}/documentation/#georeplication">.
Review comment:
lol whoops
--
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
[GitHub] [kafka] ijuma commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r662307716
##########
File path: core/src/test/scala/integration/kafka/tools/MirrorMakerIntegrationTest.scala
##########
@@ -35,6 +35,9 @@ import org.junit.jupiter.api.Test
import org.junit.jupiter.api.Assertions._
import org.junit.jupiter.api.BeforeEach
+import scala.annotation.nowarn
+
+@nowarn("cat=deprecation")
Review comment:
Can we deprecate the test too instead of the warning suppression?
--
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
[GitHub] [kafka] ryannedolan commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
Posted by GitBox <gi...@apache.org>.
ryannedolan commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r663166559
##########
File path: core/src/test/scala/integration/kafka/tools/MirrorMakerIntegrationTest.scala
##########
@@ -35,6 +35,9 @@ import org.junit.jupiter.api.Test
import org.junit.jupiter.api.Assertions._
import org.junit.jupiter.api.BeforeEach
+import scala.annotation.nowarn
+
+@nowarn("cat=deprecation")
Review comment:
good call, done!
--
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