KAFKA-5126: Implement KIP-98 transactional methods in the MockProducer #2951
Conversation
|
Call for review @apurvam @hachikuji @guozhangwang @dguy |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
| public void initTransactions() { | ||
| verifyProducerState(); | ||
| if (this.transactionInitialized) { | ||
| throw new IllegalStateException("MockedProducer got already initialized for transactions."); |
There was a problem hiding this comment.
nit: "MockProducer has already been initialized"
| verifyProducerState(); | ||
| verifyTransactionsInitialized(); | ||
| if (!transactionInFlight) { | ||
| throw new IllegalStateException("There is not open transaction to be committed."); |
| verifyProducerState(); | ||
| verifyTransactionsInitialized(); | ||
| if (!transactionInFlight) { | ||
| throw new IllegalStateException("There is not open transaction to be committed."); |
|
|
||
| verifyProducerState(); | ||
| verifyTransactionsInitialized(); | ||
| if (!transactionInFlight) { |
There was a problem hiding this comment.
ok - same thing three times. Maybe extract it to a method verifyTransactionInflight
|
|
||
| private void verifyProducerState() { | ||
| if (this.closed) { | ||
| throw new IllegalStateException("MockedProducer is already closed."); |
There was a problem hiding this comment.
MockedProducer -> MockProducer
| public void shouldThrowOnSendOffsetsToTransactionTransactionIfNoTransactionGotStarted() { | ||
| producer.initTransactions(); | ||
| try { | ||
| producer.sendOffsetsToTransaction(null, null); |
| public void shouldThrowOnCommitTransactionIfNoTransactionGotStarted() { | ||
| producer.initTransactions(); | ||
| try { | ||
| producer.commitTransaction(); |
| public void shouldThrowOnAbortTransactionIfNoTransactionGotStarted() { | ||
| producer.initTransactions(); | ||
| try { | ||
| producer.abortTransaction(); |
| } | ||
|
|
||
| @Test | ||
| public void shouldThrowOnInitTransactionIfProducerGotClosedAlready() { |
There was a problem hiding this comment.
Here and everywhere else. Got doesn't sound right. Maybe something like:
shouldThrowOnInitTransactionIfProducerIsClosed
| producer.close(); | ||
| try { | ||
| producer.initTransactions(); | ||
| fail("Should have thrown as producer got closed already"); |
There was a problem hiding this comment.
"... as producer is closed"
Same elsewhere
|
Updated this. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
No description provided.