Skip to content
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

fix: set DescribeConfigRequest Version field #1576

Merged
merged 2 commits into from
Jan 16, 2020
Merged

fix: set DescribeConfigRequest Version field #1576

merged 2 commits into from
Jan 16, 2020

Commits on Jan 16, 2020

  1. fix: set DescribeConfigRequest Version field

    Although fbd8338 had added protocol support for DescribeConfigsRequest
    v1 and v2, nothing in the admin client was actually setting the Version
    field to utilise these. Set the Version based on the selected
    sarama.Config version.
    
    Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
    dnwe committed Jan 16, 2020
    Configuration menu
    Copy the full SHA
    5de8dba View commit details
    Browse the repository at this point in the history
  2. fix: remove Logger inadvertently left in test

    This Logger modification would occasionally hit a data racein Travis:
    ```
    WARNING: DATA RACE
    Read at 0x00000130ced0 by goroutine 60:
      github.com/Shopify/sarama.(*Broker).Close()
          /home/travis/gopath/src/github.com/Shopify/sarama/broker.go:253 +0x4f0
      github.com/Shopify/sarama.safeAsyncClose.func1()
          /home/travis/gopath/src/github.com/Shopify/sarama/utils.go:52 +0xa3
      github.com/Shopify/sarama.withRecover()
          /home/travis/gopath/src/github.com/Shopify/sarama/utils.go:45 +0x84
    
    Previous write at 0x00000130ced0 by goroutine 75:
      github.com/Shopify/sarama.TestClusterAdminDescribeBrokerConfig()
          /home/travis/gopath/src/github.com/Shopify/sarama/admin_test.go:553 +0x220
      testing.tRunner()
          /home/travis/.gimme/versions/go1.12.15.linux.amd64/src/testing/testing.go:865 +0x163
    ```
    
    It had inadvertently been included in the test, so just remove it
    dnwe committed Jan 16, 2020
    Configuration menu
    Copy the full SHA
    9ba6d8e View commit details
    Browse the repository at this point in the history