Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

APEXMALHAR-2497 APEXMALHAR-2162 1) Refactor the Exactly Once output operator. 2) Refactor and fix the issues of unit tests. #642

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chaithu14
Copy link
Contributor

@tushargosavi @tweise Please review and merge.


public abstract class AbstractEmbeddedKafka
{
private static final String[] KAFKA_PATH = new String[]{"/tmp/kafka-test1","/tmp/kafka-test2"};
Copy link
Contributor

Choose a reason for hiding this comment

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

There should not be such hard coded tmp directories.

import kafka.utils.ZKStringSerializer$;
import kafka.utils.ZkUtils;

public abstract class AbstractEmbeddedKafka
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed, why not use kafka-unit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tweise kafka-unit has a limit on number of connections. It's a hard limit and doesn't have an option to increase connections.

Copy link
Contributor

@tweise tweise Sep 5, 2017

Choose a reason for hiding this comment

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

Which test is affected by that? There are several other embedded Kafka examples to look at, here is another one: https://github.com/confluentinc/examples/blob/3.2.x/kafka-streams/src/test/java/io/confluent/examples/streams/kafka/EmbeddedSingleNodeKafkaCluster.java

}
}

public static AbstractEmbeddedKafka kafkaServer;
Copy link
Contributor

@tweise tweise Jul 16, 2017

Choose a reason for hiding this comment

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

Can't this be

Rule
public KafkaUnitRule kafkaUnitRule = new KafkaUnitRule(zkPort, brokerPort);

and if multiple servers are needed create multiple rules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same comment as above.

@tweise
Copy link
Contributor

tweise commented Jul 17, 2017

Please submit separate PRs for the two JIRAs. That will simplify review and is the recommended approach in general.

Copy link
Contributor

@tweise tweise left a comment

Choose a reason for hiding this comment

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

See previous review comments.

…perator. 2) Refactor and fix the issues of unit tests.
@chaithu14 chaithu14 force-pushed the APEXMALHAR-2497-Kafka10Exactly branch from 54565e0 to 15d2903 Compare November 10, 2017 11:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants