Skip to content
This repository has been archived by the owner on Nov 20, 2024. It is now read-only.

AdminClient customizer #1023

Merged
merged 2 commits into from
Feb 3, 2021
Merged

AdminClient customizer #1023

merged 2 commits into from
Feb 3, 2021

Conversation

sobychacko
Copy link
Contributor

Provide the ability for applications to customize AdminClient by
introducing a new interface AdminClientConfigCustomizer.

Resolves #1014

Provide the ability for applications to customize AdminClient by
introducing a new interface AdminClientConfigCustomizer.

Resolves spring-attic#1014
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.

I wonder if we should add the binding name to these customizer interfaces and customize a new set of properties for each binding?

Perhaps another issue, though, for a future release.

Another issue is how can they tell which binder is being customized when named binders are being used. I suppose they can use the broker addresses to determine it, but perhaps it would be nice to pass them the binder name. Again, new feature, perhaps.

[[admin-client-config-customization]]
=== Customizing AdminClient Configuration

As with consumer and producer config customization above, applications can also customize the configuration for admin client by providing an AdminClientConfigCustomizer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
As with consumer and producer config customization above, applications can also customize the configuration for admin client by providing an AdminClientConfigCustomizer.
As with consumer and producer config customization above, applications can also customize the configuration for admin clients by providing an `AdminClientConfigCustomizer`.

Copy link
Contributor Author

@sobychacko sobychacko Feb 3, 2021

Choose a reason for hiding this comment

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

Good points about the bindings/binder names. Issue created.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant all the customizers (producer, consumer) not just the admin.

@@ -97,7 +100,7 @@ public void testAutoCreateTopicDisabledFailsOnProducerIfTopicNonExistentOnBroker
configurationProperties.getConfiguration().put("max.block.ms", "3000");

KafkaTopicProvisioner provisioningProvider = new KafkaTopicProvisioner(
configurationProperties, kafkaProperties);
configurationProperties, kafkaProperties, adminClientConfigCustomizer);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not clear why we're changing this test since we aren't verifying that the customizer was called. It's already covered by an earlier test.

Ditto the next test.

@garyrussell garyrussell merged commit 32939e3 into spring-attic:master Feb 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

unable to customize AdminClientConfig programmatically
2 participants