Kafka
  1. Kafka
  2. KAFKA-296

Update Go Client to new version of Go

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 0.7.1
    • Fix Version/s: 0.8.0
    • Component/s: clients
    • Labels:
      None

      Description

      go (http://golang.org) is close to releasing a new release of go (1.0) which requires updates to the client, in the meantime most of the go community has moved to this version.

      This change contains:

      • language changes to existing client (os.Error, time. Signals, etc)
      • removes the Makefile's (no longer used by go)
      • It also runs "go fmt" (formats it in standard go) which are most of the lines changes, from spaces to tabs.
      • updates the import path to allow for "go get" installs (don't need to get source and build)

      Not sure which versions this should apply to, but i think it should go to 0.7 and newer.

      1. kafka296_v4.patch
        92 kB
        Jeffrey Damick
      2. kafka296v3.git.patch
        33 kB
        AaronR
      3. kafka296-v2.patch
        14 kB
        AaronR
      4. go1updates.patch
        68 kB
        AaronR

        Activity

        Hide
        AaronR added a comment -

        Patches attached in separate file.

        Show
        AaronR added a comment - Patches attached in separate file.
        Hide
        Jeffrey Damick added a comment -

        good updates, however the formatting wouldn't be so big if you stuck with the current switches..

        (from the old Makefile and still work..)
        gofmt -w -tabwidth=2 -tabindent=false

        i'd rather use spaces and not tabs..

        Show
        Jeffrey Damick added a comment - good updates, however the formatting wouldn't be so big if you stuck with the current switches.. (from the old Makefile and still work..) gofmt -w -tabwidth=2 -tabindent=false i'd rather use spaces and not tabs..
        Hide
        AaronR added a comment -

        Ill submit a new patch that removes the formatting changes.

        Show
        AaronR added a comment - Ill submit a new patch that removes the formatting changes.
        Hide
        AaronR added a comment -

        updated simpler patch without fomatting changes.

        Show
        AaronR added a comment - updated simpler patch without fomatting changes.
        Hide
        Jun Rao added a comment -

        I can't seem to apply the patch. Could you rebase?
        patching file clients/go/tools/publisher/publisher.go
        Hunk #1 FAILED at 22.
        Hunk #2 succeeded at 87 with fuzz 2 (offset 38 lines).
        1 out of 2 hunks FAILED – saving rejects to file clients/go/tools/publisher/publisher.go.rej
        patching file clients/go/tools/publisher/publisher.go
        Hunk #1 FAILED at 23.
        Hunk #2 FAILED at 41.
        2 out of 2 hunks FAILED – saving rejects to file clients/go/tools/publisher/publisher.go.rej

        Show
        Jun Rao added a comment - I can't seem to apply the patch. Could you rebase? patching file clients/go/tools/publisher/publisher.go Hunk #1 FAILED at 22. Hunk #2 succeeded at 87 with fuzz 2 (offset 38 lines). 1 out of 2 hunks FAILED – saving rejects to file clients/go/tools/publisher/publisher.go.rej patching file clients/go/tools/publisher/publisher.go Hunk #1 FAILED at 23. Hunk #2 FAILED at 41. 2 out of 2 hunks FAILED – saving rejects to file clients/go/tools/publisher/publisher.go.rej
        Hide
        AaronR added a comment -

        Sorry about that, i tried to make a svn patch unsuccessfully, but this is a .git patch, i saw a few other ones so hopefully that is ok.

        Show
        AaronR added a comment - Sorry about that, i tried to make a svn patch unsuccessfully, but this is a .git patch, i saw a few other ones so hopefully that is ok.
        Hide
        Jun Rao added a comment -

        Thanks for the patch. Just committed to trunk.

        Show
        Jun Rao added a comment - Thanks for the patch. Just committed to trunk.
        Hide
        Jun Rao added a comment -

        Actually, I have to revert the commit since the patch didn't apply cleanly.

        ! clients/go/tools/offsets/Makefile
        M clients/go/tools/offsets/offsets.go
        M clients/go/tools/consumer/consumer.go
        ! clients/go/tools/consumer/Makefile
        M clients/go/tools/publisher/publisher.go
        ! clients/go/tools/publisher/Makefile
        ! clients/go/kafka_test.go

        Show
        Jun Rao added a comment - Actually, I have to revert the commit since the patch didn't apply cleanly. ! clients/go/tools/offsets/Makefile M clients/go/tools/offsets/offsets.go M clients/go/tools/consumer/consumer.go ! clients/go/tools/consumer/Makefile M clients/go/tools/publisher/publisher.go ! clients/go/tools/publisher/Makefile ! clients/go/kafka_test.go
        Hide
        Jun Rao added a comment -

        Also, could you add some basic unit tests for the go client?

        Show
        Jun Rao added a comment - Also, could you add some basic unit tests for the go client?
        Hide
        Jeffrey Damick added a comment -

        this patch applies cleanly to svn & moves some of the files to fit better with latest go style.
        I temporarily removed the remote import so that it would compile the standalone apps (appears to be a bit of chicken & egg problem).

        Show
        Jeffrey Damick added a comment - this patch applies cleanly to svn & moves some of the files to fit better with latest go style. I temporarily removed the remote import so that it would compile the standalone apps (appears to be a bit of chicken & egg problem).
        Hide
        AaronR added a comment -

        that version looks and works for me. Not sure why you were getting errors on the standalone apps using the remote import, as that was working for me. The remote install can be a future change. If we can get this closed then I can resubmit #297, #298, and a #314 ( MultiProduce ) .

        Also, we probably should update the docs to include notes about having to modify GOPATH etc since it is not go get installable. I can add that in #297.

        Show
        AaronR added a comment - that version looks and works for me. Not sure why you were getting errors on the standalone apps using the remote import, as that was working for me. The remote install can be a future change. If we can get this closed then I can resubmit #297, #298, and a #314 ( MultiProduce ) . Also, we probably should update the docs to include notes about having to modify GOPATH etc since it is not go get installable. I can add that in #297.
        Hide
        Jeffrey Damick added a comment - - edited

        thanks, yeah i did add comments to the README about setting GOPATH.

        Jun, can you verify & commit please.

        Show
        Jeffrey Damick added a comment - - edited thanks, yeah i did add comments to the README about setting GOPATH. Jun, can you verify & commit please.
        Hide
        David Arthur added a comment -

        I thought we were moving clients out of the main project?

        Show
        David Arthur added a comment - I thought we were moving clients out of the main project?
        Hide
        Jeffrey Damick added a comment -

        this is a very old ticket that was never completed.

        Show
        Jeffrey Damick added a comment - this is a very old ticket that was never completed.

          People

          • Assignee:
            Unassigned
            Reporter:
            AaronR
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development