Skip to content

Conversation

artembilan
Copy link
Member

Fixes #1008

  • If provided configs for the DefaultKafkaProducerFactory contains a
    ProducerConfig.TRANSACTIONAL_ID_CONFIG value, treat it as a prefix
    and override for the target Producer configs.
    Also log an INFO that this value is going to be prefix the target
    Producer
  • An explicit setTransactionIdPrefix() will override this value
    any way
  • Polishing for KafkaTemplateTransactionTests together with an
    explicit ProducerConfig.TRANSACTIONAL_ID_CONFIG config in the
    testLocalTransaction() test

Fixes spring-projects#1008

* If provided `configs` for the `DefaultKafkaProducerFactory` contains a
`ProducerConfig.TRANSACTIONAL_ID_CONFIG` value, treat it as a `prefix`
and override for the target `Producer` configs.
Also log an `INFO` that this value is going to be prefix the target
`Producer`
* An explicit `setTransactionIdPrefix()` will override this value
any way
* Polishing for `KafkaTemplateTransactionTests` together with an
explicit `ProducerConfig.TRANSACTIONAL_ID_CONFIG` config in the
`testLocalTransaction()` test
Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

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

Just one issue.


String txId = (String) this.configs.get(ProducerConfig.TRANSACTIONAL_ID_CONFIG);
if (StringUtils.hasText(txId)) {
setTransactionIdPrefix(txId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to be made final when calling from a CTOR. Perhaps add a protected getter if a subclass wants to sniff the value.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is there. See the change on line 186.

I'm OK to add getter.

Copy link
Contributor

Choose a reason for hiding this comment

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

Doh, I missed it because of all the other noise 😦 sorry.

@garyrussell garyrussell merged commit 01154a9 into spring-projects:master Apr 11, 2019
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.

ProducerConfig.TRANSACTIONAL_ID_CONFIG and ProducerFactory.transactionIdPrefix overlapping ?

2 participants