Kafka
  1. Kafka
  2. KAFKA-1308

Publish jar of test utilities to Maven

    Details

    • Type: Wish Wish
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.8.1
    • Fix Version/s: 0.8.2.0
    • Component/s: None
    • Labels:
      None

      Description

      For projects that use Kafka, and want to write tests that exercise Kafka (in our case, Samza), it's useful to have access to Kafka's test utility classes such as kafka.zk.EmbeddedZookeeper and kafka.utils.TestUtils. We can use ./gradlew testJar to build jar files that contain those classes, but as far as I know, these are currently not made available in a binary release.

      At the moment, we have to check those kafka*-test.jar files into the Samza repository. To avoid that, would it be possible to publish those jars of tests to Maven, so that they fit into the normal dependency management?

      Or perhaps, if publishing the tests themselves is not appropriate, we could move the test utilities into a separate module that is published, and make the tests depend on that module?

      1. kafka-1308_v2_0.8.1.patch
        0.5 kB
        Jun Rao
      2. KAFKA-1308.patch
        2 kB
        Jakob Homan
      3. KAFKA-1308.patch
        3 kB
        Chris Campbell
      4. KAFKA-1308-2.patch
        0.8 kB
        Jakob Homan
      5. KAFKA-1308-8.1.1.patch
        2 kB
        Jakob Homan
      6. KAFKA-1308-trunk.patch
        0.5 kB
        Jakob Homan

        Issue Links

          Activity

          Hide
          Chris Campbell added a comment -

          I'm attaching a small patch (see KAFKA-1308.patch) that adds a trivial testutils project containing only the kafka.utils.TestUtils class. I'm not too experienced with Gradle so I don't know if this is the best approach (see the TODO), but this seems to get the job done. Verified with our in-house Maven repo.

          Show
          Chris Campbell added a comment - I'm attaching a small patch (see KAFKA-1308 .patch) that adds a trivial testutils project containing only the kafka.utils.TestUtils class. I'm not too experienced with Gradle so I don't know if this is the best approach (see the TODO), but this seems to get the job done. Verified with our in-house Maven repo.
          Hide
          Yuta Okamoto added a comment -

          +1

          We need kafka*-test.jar for testing for our third-party client, finagle-kafka, without adding the jar to the repository.

          Show
          Yuta Okamoto added a comment - +1 We need kafka*-test.jar for testing for our third-party client, finagle-kafka , without adding the jar to the repository.
          Hide
          Mikael Högqvist added a comment -

          +1

          TestUtils are very useful for us as well.

          Show
          Mikael Högqvist added a comment - +1 TestUtils are very useful for us as well.
          Hide
          Ivan Lyutov added a comment -

          +1
          Patch looks nice. It might be very useful.

          Show
          Ivan Lyutov added a comment - +1 Patch looks nice. It might be very useful.
          Hide
          Jun Rao added a comment -

          Thanks for the patch. I am not sure if it's worthwhile to create a separate jar just for TestUtils. There could be other classes that people may want to use. It seems that we just need to publish the test jar to maven.

          Joe Stein,

          Do you think you can upload the test jars for 0.8.1.1 to maven? Thanks,

          Show
          Jun Rao added a comment - Thanks for the patch. I am not sure if it's worthwhile to create a separate jar just for TestUtils. There could be other classes that people may want to use. It seems that we just need to publish the test jar to maven. Joe Stein, Do you think you can upload the test jars for 0.8.1.1 to maven? Thanks,
          Hide
          Chris Campbell added a comment -

          For what its worth, the main reason I created a separate jar for the patch was this message, which seemed to imply that it's not useful to ship the test cases:
          http://mail-archives.apache.org/mod_mbox/kafka-users/201308.mbox/%3CCAOeJiJgJpPc+m+XQeYRNiNjcMq9VSJfzp3eFhvAKNbDjndLj3Q@mail.gmail.com%3E

          If there are other classes that people want to use, it seems better to include them in this separately published jar to help make it clear what is meant to be fit for public consumption, rather than just a wholesale dump of test classes. But it's not a big deal either way

          Show
          Chris Campbell added a comment - For what its worth, the main reason I created a separate jar for the patch was this message, which seemed to imply that it's not useful to ship the test cases: http://mail-archives.apache.org/mod_mbox/kafka-users/201308.mbox/%3CCAOeJiJgJpPc+m+XQeYRNiNjcMq9VSJfzp3eFhvAKNbDjndLj3Q@mail.gmail.com%3E If there are other classes that people want to use, it seems better to include them in this separately published jar to help make it clear what is meant to be fit for public consumption, rather than just a wholesale dump of test classes. But it's not a big deal either way
          Hide
          Jakob Homan added a comment -

          Patches for both trunk and 8.1.1 to include the test jars when publishing. Have tested locally but not pushed yet.

          Show
          Jakob Homan added a comment - Patches for both trunk and 8.1.1 to include the test jars when publishing. Have tested locally but not pushed yet.
          Hide
          Jakob Homan added a comment -

          The patches are identical, just slight differing in line numbers. Git applied the 8.1.1 to trunk with no problems, but still made two patches for clarity.

          Show
          Jakob Homan added a comment - The patches are identical, just slight differing in line numbers. Git applied the 8.1.1 to trunk with no problems, but still made two patches for clarity.
          Hide
          Jakob Homan added a comment -
          Show
          Jakob Homan added a comment - rb: https://reviews.apache.org/r/22802/
          Hide
          Jun Rao added a comment -

          Attached is a simpler patch for 0.8.1. I do see the test jar copied over to maven local when running "./gradlew uploadArchives". I don't see a pom file for the test jar though. Not sure if this needed.

          Show
          Jun Rao added a comment - Attached is a simpler patch for 0.8.1. I do see the test jar copied over to maven local when running "./gradlew uploadArchives". I don't see a pom file for the test jar though. Not sure if this needed.
          Hide
          Jakob Homan added a comment -

          Yes, we need the pom.

          Show
          Jakob Homan added a comment - Yes, we need the pom.
          Hide
          Jakob Homan added a comment -

          Looks like this works without the pom. I've updated Jun's patch to change the Kafka version string to be SNAPSHOTed, since everything but an RC should be a snapshot. As part of testing, I published snapshots of 0.8.1.1 snapshots and used them against SAMZA (SAMZA-284).

          Show
          Jakob Homan added a comment - Looks like this works without the pom. I've updated Jun's patch to change the Kafka version string to be SNAPSHOTed, since everything but an RC should be a snapshot. As part of testing, I published snapshots of 0.8.1.1 snapshots and used them against SAMZA ( SAMZA-284 ).
          Hide
          Neha Narkhede added a comment -

          +1 on KAFKA-1308-v2.patch. Thanks Jakob!

          Show
          Neha Narkhede added a comment - +1 on KAFKA-1308 -v2.patch. Thanks Jakob!
          Hide
          Jakob Homan added a comment -

          We're still blocked on the Samza release even with this patch because the snapshots I'm publishing are being cleaned on a daily basis. To get around this I would like to publish the 0.8.1.1 release to the main maven site using this patch:

          This is ASF-ok since this patch does not change anything about the code that was generated, just publishes an extra jar on top of what was previously published. This will unblock us since we can then pull from the main repo.

          The alternative is to wait for Kafka to release a 0.8.1.2 (or whatever), which would add an extra week or more for the vote to run.

          Show
          Jakob Homan added a comment - We're still blocked on the Samza release even with this patch because the snapshots I'm publishing are being cleaned on a daily basis. To get around this I would like to publish the 0.8.1.1 release to the main maven site using this patch: Use the rc1 tgz that was voted in as 0.8.1.1 as the release ( https://people.apache.org/~joestein/kafka-0.8.1.1-candidate1/kafka-0.8.1.1-src.tgz ) Apply the patch, remove the -SNAPSHOT Run ./gradlew uploadArchivesAll against the main repo, publishing the test jar and all of the original jars. This is ASF-ok since this patch does not change anything about the code that was generated, just publishes an extra jar on top of what was previously published. This will unblock us since we can then pull from the main repo. The alternative is to wait for Kafka to release a 0.8.1.2 (or whatever), which would add an extra week or more for the vote to run.
          Hide
          Joe Stein added a comment -

          I am +1 to Jakob's suggestion and I can do this in a couple of hours if others agree (not sure we need a vote I think we are good with just the commit and upload release what is there no code change)

          Show
          Joe Stein added a comment - I am +1 to Jakob's suggestion and I can do this in a couple of hours if others agree (not sure we need a vote I think we are good with just the commit and upload release what is there no code change)
          Hide
          Joe Stein added a comment -

          which patch(s) would apply to the 0.8.1.1 tag?

          it looks like I would apply KAFKA-1308-8.1.1.patch and then kafka-1308_v2_0.8.1.patch

          also I don't see the SNAPSHOT change in either patch it looks like only in KAFKA-1308-2.patch

          just so we are all saying the same thing we are expecting

          Show
          Joe Stein added a comment - which patch(s) would apply to the 0.8.1.1 tag? it looks like I would apply KAFKA-1308 -8.1.1.patch and then kafka-1308_v2_0.8.1.patch also I don't see the SNAPSHOT change in either patch it looks like only in KAFKA-1308 -2.patch just so we are all saying the same thing we are expecting
          Hide
          Neha Narkhede added a comment -

          I'm also +1 on Jakob's suggestion. Joe, thanks for picking it up.

          Show
          Neha Narkhede added a comment - I'm also +1 on Jakob's suggestion. Joe, thanks for picking it up.
          Hide
          Jakob Homan added a comment -

          Yes, just KAFKA-1308-2.patch (and then manually remove the -SNAPSHOT) designation. This patch has been +1'ed by Neha and I intend to commit it to trunk and the 0.8.1.1 branch.

          I've been using this patch to publish snapshots and testing those against Samza, so I'm confident the patch itself is good.

          Show
          Jakob Homan added a comment - Yes, just KAFKA-1308 -2.patch (and then manually remove the -SNAPSHOT) designation. This patch has been +1'ed by Neha and I intend to commit it to trunk and the 0.8.1.1 branch. I've been using this patch to publish snapshots and testing those against Samza, so I'm confident the patch itself is good.
          Hide
          Jakob Homan added a comment -

          If no one is opposed, I'll go ahead and do it now.

          Show
          Jakob Homan added a comment - If no one is opposed, I'll go ahead and do it now.
          Hide
          Joe Stein added a comment -

          Sounds good, I will wait for the commit to the 0.8.1 branch and then push the jar to maven staging , then will post here again for your +1 before shipping to central

          Show
          Joe Stein added a comment - Sounds good, I will wait for the commit to the 0.8.1 branch and then push the jar to maven staging , then will post here again for your +1 before shipping to central
          Hide
          Jakob Homan added a comment -

          I've committed this to 0.8.1. I had already staged the jars, so I just went ahead and promoted the release to central as well.

          Show
          Jakob Homan added a comment - I've committed this to 0.8.1. I had already staged the jars, so I just went ahead and promoted the release to central as well.
          Hide
          Jun Rao added a comment -

          Jakob,

          Do you plan to commit the patch to trunk too?

          Thanks,

          Show
          Jun Rao added a comment - Jakob, Do you plan to commit the patch to trunk too? Thanks,
          Hide
          Jakob Homan added a comment -

          Yes, had a merge conflict when I went to do so; will resolve tomorrow and commit.

          Show
          Jakob Homan added a comment - Yes, had a merge conflict when I went to do so; will resolve tomorrow and commit.
          Hide
          Jakob Homan added a comment -

          Resolved conflict (in version number; also dropped the -INCUBATING). Attaching trunk version of patch. Committed to trunk. Resolving.

          Show
          Jakob Homan added a comment - Resolved conflict (in version number; also dropped the -INCUBATING). Attaching trunk version of patch. Committed to trunk. Resolving.

            People

            • Assignee:
              Jakob Homan
              Reporter:
              Martin Kleppmann
            • Votes:
              4 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development