Skip to content
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

cleanup for config server #1615

Merged
merged 148 commits into from
Mar 29, 2024

Conversation

wind57
Copy link
Contributor

@wind57 wind57 commented Mar 28, 2024

No description provided.

wind57 and others added 30 commits December 4, 2021 07:59
@@ -117,16 +117,6 @@ static MultipleSourcesContainer secretsDataByName(CoreV1Api coreV1Api, String na
includeDefaultProfileData);
}

static MultipleSourcesContainer secretsDataByName(CoreV1Api coreV1Api, String namespace,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are never used

@@ -194,11 +194,11 @@ public static MultipleSourcesContainer processNamedData(List<StrippedSourceConta
// processed before profile based sources. This way, we replicate that
// "application-dev.yaml"
// overrides properties from "application.yaml"
sourceNames.forEach(source -> {
Copy link
Contributor Author

@wind57 wind57 Mar 28, 2024

Choose a reason for hiding this comment

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

I want to present the minor changes I have in mind in small batches, so that it's easier to review, so a few more like this will follow.

What I am doing here is rename source to sourceName; p with activeProfile; and replace contains with endsWith:

p -> key.contains("-" + p)

to

sourceName.endsWith("-" + activeProfile)

@wind57 wind57 changed the title Some cleanup for config server 2 cleanup for config server Mar 28, 2024
@wind57 wind57 marked this pull request as ready for review March 29, 2024 18:02
@wind57
Copy link
Contributor Author

wind57 commented Mar 29, 2024

@ryanjbaxter a few of these will come... thank you for looking at it

@@ -230,7 +231,8 @@ public static MultipleSourcesContainer processNamedData(List<StrippedSourceConta
*/
private static boolean containsDataWithProfile(Map<String, String> rawData, String[] activeProfiles) {
return rawData.keySet().stream().anyMatch(
key -> Arrays.stream(activeProfiles).anyMatch(p -> key.contains("-" + p) || "default".equals(p)));
key -> Arrays.stream(activeProfiles).anyMatch(activeProfile -> key.contains("-" + activeProfile)
Copy link
Contributor

Choose a reason for hiding this comment

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

Sholdn't this be endsWith as well instead of contains?

Copy link
Contributor Author

@wind57 wind57 Mar 29, 2024

Choose a reason for hiding this comment

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

not here no, good eye though :) in the case above we check the source name (config map name), so its something like : account-k8s; so endsWith is perfectly valid.

In the case here, we check for -<ACTIVE_PROFILE>.<EXTENSTION>, where extension is yaml/yml/properties, so it makes sense to be contains here, for the time being at least.

In my next PR, I will amend this a bit too.

@wind57 wind57 requested a review from ryanjbaxter March 29, 2024 19:33
@ryanjbaxter ryanjbaxter added this to the 3.1.2 milestone Mar 29, 2024
@ryanjbaxter ryanjbaxter merged commit 5b201f0 into spring-cloud:main Mar 29, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants