Aias00 commented on PR #7917:
URL: https://github.com/apache/shenyu/pull/7917#issuecomment-6333799074
Friendly nudge — the branch is still at `6ef826af`, so nothing has changed
since my review.
The one blocking item is small and self-contained: the PR makes a
pre-registered `DeploymentProperties` bean fatal.
```
T6 new | props pre-registered (explicit name) -> STARTUP FAILED:
NoUniqueBeanDefinitionException : expected single matching bean but found
2:
preRegisteredK8sProperties,
shenyu.k8s.deployment-org.apache.shenyu.admin.config.properties.DeploymentProperties
```
Today's `shouldCreateClientEvenWhenPropertiesBeanIsPreRegistered` passes
only because the harness registers the bean under the name
`deploymentProperties`, which happens to equal the factory method's parameter
name — with `-parameters` enabled, Spring resolves the injection by name and
the test goes green while a real user's explicitly-named properties bean fails
the context. Renaming the pre-registered bean reproduces the startup failure.
The fix I suggested still holds: declare the properties bean yourself
instead of relying on `@EnableConfigurationProperties` ordering,
```java
@Bean
@ConditionalOnMissingBean
@ConfigurationProperties(prefix = shenyu.k8s.deployment)
public DeploymentProperties deploymentProperties() {
return new DeploymentProperties();
}
```
and swap the `containsBean(apiClient)` assertion for
`getBeansOfType(AppsV1Api.class)`, since `containsBean` cannot tell a created
bean from a `NullBean` produced by `return null`.
The non-blocking notes from the same review (PR-body wording about the
condition never passing, `scaleByAction` dropping `scaleByNum`'s boolean,
`Config.fromToken(..., false)` disabling TLS verification while also setting a
CA, and the `:`/`=` mix in `enabledProps()`) can be taken or left — but the
description claim about the condition is worth correcting since it is what the
reviewer will check first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
