Skip to content

Add config property for KafkaAdmin modifyTopicConfigs #31679

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

Conversation

m-kay
Copy link
Contributor

@m-kay m-kay commented Jul 12, 2022

Adds a new configuration property spring.kafka.admin.modify-topic-configs to facilitate enabling of this recently added functionality

@m-kay m-kay changed the base branch from main to 2.7.x July 12, 2022 06:27
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 12, 2022
@wilkinsona
Copy link
Member

Thanks for the proposal. We can consider this for 3.0.x but it's too late now for 2.7.x, particular given that it's only a few lines of code to define your own KafkaAdmin bean:

public KafkaAdmin kafkaAdmin(KafkaProperties properties) {
	KafkaAdmin kafkaAdmin = new KafkaAdmin(properties.buildAdminProperties());
	kafkaAdmin.setFatalIfBrokerNotAvailable(properties.getAdmin().isFailFast());
	kafkaAdmin.setModifyTopicConfigs(true);
	return kafkaAdmin;
}

@garyrussell do you think it makes sense to surface a configuration property for this setting?

@wilkinsona wilkinsona added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Jul 12, 2022
@m-kay
Copy link
Contributor Author

m-kay commented Jul 12, 2022

@wilkinsona The reason I would have liked it as configuration property is to make it easily configurable in a k8s environment.

@wilkinsona
Copy link
Member

@m-kay Until Boot has a property, you can externalise it using your own property. You could even name that property spring.kafka.admin.modify-topic-configs if you were prepared to deal with a potential clash in the future.

@m-kay
Copy link
Contributor Author

m-kay commented Jul 12, 2022

Yes sure I can do that. I just thought I could also help others with this small change here 😉
Besides that I like to use spring boots auto configuration wherever possible, instead of creating the beans in my own config, to benefit from new configurations added in future releases.

@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Jul 12, 2022
@snicoll snicoll added this to the 3.0.x milestone Jul 12, 2022
@wilkinsona wilkinsona added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Jul 12, 2022
@garyrussell
Copy link
Contributor

Yes, it makes sense to add it. Perhaps a simpler work around (without having to add your own KafkaAdmin bean) is to set the property in one of the NewTopic bean definitions.

	@Bean
	public NewTopic topic(KafkaAdmin admin) {
		admin.setModifyTopicConfigs(true);
		return TopicBuilder.name("topic1")
				.partitions(1)
				.replicas(1)
				.config("retention.ms", "60000")
				.build();
	}

@wilkinsona
Copy link
Member

Thanks, Gary. We'll add the property in 3.0.

FWIW, that mutation feels a little risky to me as its ordering with respect to other usages of KafkaAdmin is undefined. It may be fine given how Spring Kafka uses the KafkaAdmin bean but, generally speaking, I wouldn't recommend a @Bean method mutating another bean.

@wilkinsona wilkinsona removed the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Jul 12, 2022
@garyrussell
Copy link
Contributor

I agree, in general, but in this case it's safe because KafkaAdmin is a SmartInitializingSingleton so it doesn't do any work until all the beans have been instantiated.

@snicoll snicoll changed the base branch from 2.7.x to main July 13, 2022 12:05
@snicoll snicoll self-assigned this Jul 13, 2022
@snicoll snicoll modified the milestones: 3.0.x, 3.0.0-M4 Jul 13, 2022
@snicoll snicoll closed this in a2ccaa2 Jul 13, 2022
@snicoll
Copy link
Member

snicoll commented Jul 13, 2022

@m-kay thank you for making your first contribution to Spring Boot.

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.

5 participants