Fix kafka connection string auth configuration condition and improve BPP #43854
Conversation
|
API change check API changes are not detected in this pull request. |
...ing/cloud/autoconfigure/implementation/eventhubs/kafka/KafkaPropertiesBeanPostProcessor.java
Show resolved
Hide resolved
|
|
||
| @SuppressWarnings("unchecked") | ||
| @Override | ||
| public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException { | ||
| if (needsPostProcess(bean)) { | ||
| azureGlobalProperties = applicationContext.getBean(AzureGlobalProperties.class); |
There was a problem hiding this comment.
do we need to handle the case when the bean is not present?
There was a problem hiding this comment.
the getBean will throw exception when not found the global properties.
There was a problem hiding this comment.
But the other two BPP won't throw any exceptions, so do we need to throw the exception here?
| @Role(BeanDefinition.ROLE_INFRASTRUCTURE) | ||
| static BeanPostProcessor kafkaPropertiesBeanPostProcessor(AzureGlobalProperties properties) { | ||
| return new KafkaPropertiesBeanPostProcessor(properties); | ||
| static BeanPostProcessor kafkaPropertiesBeanPostProcessor() { |
There was a problem hiding this comment.
does this mean there will be two kafka properties bpp in the context?
There was a problem hiding this comment.
Yes, one for jaas based on conn string, another for jaas based on OAuth2.
There was a problem hiding this comment.
Are they exclusive? Will they be in the context at the same time?
...ing/cloud/autoconfigure/implementation/eventhubs/kafka/KafkaPropertiesBeanPostProcessor.java
Outdated
Show resolved
Hide resolved
...ing/cloud/autoconfigure/implementation/eventhubs/kafka/KafkaPropertiesBeanPostProcessor.java
Outdated
Show resolved
Hide resolved
|
|
||
| @SuppressWarnings("unchecked") | ||
| @Override | ||
| public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException { | ||
| if (needsPostProcess(bean)) { | ||
| azureGlobalProperties = applicationContext.getBean(AzureGlobalProperties.class); |
There was a problem hiding this comment.
But the other two BPP won't throw any exceptions, so do we need to throw the exception here?
| @Role(BeanDefinition.ROLE_INFRASTRUCTURE) | ||
| static BeanPostProcessor kafkaPropertiesBeanPostProcessor(AzureGlobalProperties properties) { | ||
| return new KafkaPropertiesBeanPostProcessor(properties); | ||
| static BeanPostProcessor kafkaPropertiesBeanPostProcessor() { |
There was a problem hiding this comment.
Are they exclusive? Will they be in the context at the same time?
…azure/spring/cloud/autoconfigure/implementation/eventhubs/kafka/KafkaPropertiesBeanPostProcessor.java Co-authored-by: Xiaolu Dai <31124698+saragluna@users.noreply.github.com>
…azure/spring/cloud/autoconfigure/implementation/eventhubs/kafka/KafkaPropertiesBeanPostProcessor.java Co-authored-by: Xiaolu Dai <31124698+saragluna@users.noreply.github.com>
Description
Fixes #43853
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines