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

KAFKA-3080: Fix ConsoleConsumerTest by checking version when service is started #770

Conversation

ewencp
Copy link
Contributor

@ewencp ewencp commented Jan 14, 2016

The MessageFormatter being used was only introduced as of 0.9.0.0. The Kafka
version in some tests is changed dynamically, sometimes from trunk back to an
earlier version, so this option must be set based on the version used when the
service is started, not when it is created.

…is started

The MessageFormatter being used was only introduced as of 0.9.0.0. The Kafka
version in some tests is changed dynamically, sometimes from trunk back to an
earlier version, so this option must be set based on the version used when the
service is started, not when it is created.
@ewencp
Copy link
Contributor Author

ewencp commented Jan 14, 2016

@granders Tracked this down to 9220df9. Verified this fixes the failing test and still passes all of security_rolling_upgrade_test. Maybe @benstopford can sanity check this as well since he was the author on the original patch?

And perhaps @gwenshap for final review + commit?

@ijuma
Copy link
Contributor

ijuma commented Jan 14, 2016

Thanks for fixing this @ewencp, makes sense to me.

@granders
Copy link
Contributor

Looks good, passes locally.

Not expecting anything new, but branch builder is running here:
http://jenkins.confluent.io/job/kafka_system_tests_branch_builder/338/

@ijuma
Copy link
Contributor

ijuma commented Jan 18, 2016

@granders there were some failures in the branch builder. Are they related?

@granders
Copy link
Contributor

@ijuma
There were 2 failures, but as far as I can tell they seem unrelated:

This failed, but also failed on trunk on 1/17/2016, so I believe it is unrelated

Module: kafkatest.tests.zookeeper_security_upgrade_test
Class:  ZooKeeperSecurityUpgradeTest
Method: test_zk_security_upgrade
Arguments:
{
  "security_protocol": "SASL_PLAINTEXT"
}

This also failed

Module: kafkatest.tests.benchmark_test
Class:  Benchmark
Method: test_consumer_throughput
Arguments:
{
  "security_protocol": "SSL"
}

Failure was due to a parsing error (parsing statistics choked on a NotLeaderForPartitionException), but the same test with "interbroker_security_protocol": "PLAINTEXT", "security_protocol": "SSL" passed. If the failure was due to Ewen's change, I would have expected it to fail on "interbroker_security_protocol": "PLAINTEXT" as well.

@granders
Copy link
Contributor

@ijuma
Copy link
Contributor

ijuma commented Jan 19, 2016

Thanks @granders. It would be great to get this in @gwenshap as it fixes a bunch of system test failures.

@granthenke
Copy link
Member

LGTM. It would be great to get this in.

@gwenshap
Copy link
Contributor

LGTM

@asfgit asfgit closed this in 9577dc2 Jan 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants