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

GH-7646: Additional Kafka Properties #7672

Closed
wants to merge 3 commits into from

Conversation

garyrussell
Copy link
Contributor

Resolves #7646

Add max.poll.records and generic properties.*.

Resolves spring-projects#7646

Add `max.poll.records` and generic `properties.*`.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 16, 2016
@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 16, 2016
@philwebb philwebb added this to the 1.5.0 M1 milestone Dec 16, 2016
Since kafka properties are generally dotted, add a test to ensure
dotted "catch-all" properties work.
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Minor concern in Docs.
Otherwise LGTM

Map<String, Object> producerProperties = properties.buildProducerProperties();
producerProperties.put("some.property", "some.value");
return new DefaultKafkaProducerFactory<Object, Object>(producerProperties);
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this sample should be still here, although re-phrased.
There are some cases which aren't covered with the current KafkaProperties , (e.g. CommonClientConfigs.METRIC_REPORTER_CLASSES_CONFIG), but I definitely may be interested in different values for consumer and for producer respectively.

Copy link
Member

Choose a reason for hiding this comment

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

Can we put this code in a file in https://github.com/spring-projects/spring-boot/tree/master/spring-boot-docs/src/main/java/org/springframework/boot and then include it in the docs please? This commit provides an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sweet - will do 😄

Add back custom config example, using include.
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

👍

@snicoll snicoll self-assigned this Dec 19, 2016
philwebb pushed a commit that referenced this pull request Dec 21, 2016
Add support for arbitrary Kafka properties via
`spring.kafka.properties.*` and also a `spring.kafka.max.poll.records`
property.

See gh-7672
philwebb added a commit that referenced this pull request Dec 21, 2016
* pr/7672:
  Polish Kafka properties
  Support arbitrary Kafka properties
@philwebb philwebb closed this in 1f7b3ca Dec 21, 2016
garyrussell added a commit to garyrussell/spring-boot that referenced this pull request Jul 17, 2017
PR spring-projects#7672 Added support for arbitrary common properties.

However, Kafka emits a warning if a producer configuration contains properties intended
only for consumers, and vice versa.

The documentation showed a sample of how to write code to configure arbitrary properties
but this is inconvenient.

Add arbitrary properties to the consumer and procucer configs.
snicoll pushed a commit that referenced this pull request Jul 19, 2017
PR #7672 Added support for arbitrary common properties.

However, Kafka emits a warning if a producer configuration contains
properties intended only for consumers, and vice versa.

The documentation showed a sample of how to write code to configure
arbitrary properties but this is inconvenient.

Add arbitrary properties to the consumer and procucer configs.

See gh-9775
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants