-
Notifications
You must be signed in to change notification settings - Fork 1k
drop some deprecations #1995
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
base: main
Are you sure you want to change the base?
drop some deprecations #1995
Conversation
16611ed
to
86ee5de
Compare
@@ -24,7 +24,7 @@ runs: | |||
- name: build project | |||
shell: bash | |||
run: | | |||
./mvnw clean install -Dspring-boot.build-image.skip=true -DskipITs -DskipTests -T1C -U -B -q | |||
./mvnw clean install -P 'run-on-github-actions' -Dspring-boot.build-image.skip=true -DskipITs -DskipTests -T1C -U -B -q |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we only need this for github, so add the profile
* {@link org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties} | ||
* in the next major release. | ||
*/ | ||
@Deprecated(forRemoval = true) | ||
@Bean | ||
@ConditionalOnMissingBean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we were getting something from the environment
via :
environment.getProperty("spring.cloud.kubernetes.client.user-agent"
and now we do it from KubernetesClientProperties
. It should have been like this from the beginning, but because of other factors and being public
, we never did this clean-up.
@@ -499,8 +499,7 @@ static class LocalTestConfig { | |||
@Bean | |||
KubernetesClientProperties kubernetesClientProperties() { | |||
return new KubernetesClientProperties(null, null, null, "default", null, null, null, null, null, null, null, | |||
null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, | |||
null); | |||
null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one less argument here, rollingTimeout
was deprecated in fabric8 and not used anymore
@@ -82,46 +81,17 @@ public class KubernetesInformerDiscoveryClient implements DiscoveryClient { | |||
|
|||
private final ServicePortSecureResolver servicePortSecureResolver; | |||
|
|||
// visible only for testing and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment is first resolved, related to CoreV1Api
CoreV1Api coreV1Api; | ||
|
||
@Deprecated(forRemoval = true) | ||
public KubernetesInformerDiscoveryClient(String namespace, SharedInformerFactory sharedInformerFactory, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this constructor is removed, and we are left with a single one.
Let me try to simplify this. We had two constructors:
public KubernetesInformerDiscoveryClient(List<SharedInformerFactory> factories) {
}
and
public KubernetesInformerDiscoveryClient(SharedInformerFactory factory) {
}
again, this simplifies it so that its easier to understand the change.
Now, we only have a single constructor, that accepts a List
. As such, all the callers we had, have to change now.
@@ -33,9 +33,9 @@ public record KubernetesClientProperties(Boolean trustCerts, String masterUrl, S | |||
String caCertFile, String caCertData, String clientCertFile, String clientCertData, String clientKeyFile, | |||
String clientKeyData, String clientKeyAlgo, String clientKeyPassphrase, String username, String password, | |||
Duration watchReconnectInterval, Duration watchReconnectLimit, Duration connectionTimeout, | |||
Duration requestTimeout, @Deprecated(forRemoval = true) Duration rollingTimeout, Duration loggingInterval, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one @Deprecated(forRemoval = true) Duration rollingTimeout
is dropped
5dd7054
to
31b10cc
Compare
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
@ryanjbaxter this one is ready also and is a little more involved. But its only "involved" because of the number of files, because all it does is drop the deprecated code and adjust according to that. I left some comments where I thought that the changes are most relevant. Thank you |
No description provided.