Skip to content

Kafka 3078#747

Closed
SinghAsDev wants to merge 1 commit into
apache:trunkfrom
SinghAsDev:KAFKA-3078
Closed

Kafka 3078#747
SinghAsDev wants to merge 1 commit into
apache:trunkfrom
SinghAsDev:KAFKA-3078

Conversation

@SinghAsDev

Copy link
Copy Markdown
Contributor

Note that KAFKA-3077 will be required to run these tests.

@SinghAsDev

Copy link
Copy Markdown
Contributor Author

@granders @ijuma mind taking a look?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal, but for future reference doing something like if self.security_protocol in [SecurityConfig.SASL_PLAINTEXT, SecurityConfig.SASL_SSL, SecurityConfig.SASL_MECHANISM_GSSAPI, SecurityConfig.SASL_MECHANISM_PLAIN] is a more concise way to express conditions like this. In fact, it might be valuable to add those collections to SecurityConfig since it would allow for simple tests that look something like if self.security_protocol in SecurityConfig.SASL_MECHANISMS.

@ewencp

ewencp commented Jan 12, 2016

Copy link
Copy Markdown
Contributor

LGTM. I know you filed the ducktape test as a separate subtask, but in the future if you want to roll the system test update into the same patch, it's actually easier to review together for relatively small patches like this since the system test itself is the best validation of the original patch.

@asfgit asfgit closed this in 3e5afbf Jan 12, 2016
AnGg98 pushed a commit to AnGg98/kafka that referenced this pull request Jul 25, 2022
With 6f81bb1 merged, we can now use c4.xlarge instances with our system tests, which are much more reliable for spot than m3.xlarge.
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.

2 participants