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

Fix 1338 main #1344

Merged
merged 58 commits into from
May 12, 2023
Merged

Fix 1338 main #1344

merged 58 commits into from
May 12, 2023

Conversation

wind57
Copy link
Contributor

@wind57 wind57 commented May 12, 2023

No description provided.

wind57 and others added 30 commits December 4, 2021 07:59
wind57 added 22 commits April 8, 2023 16:34
@@ -43,13 +43,14 @@ public ConfigData load(ConfigDataLoaderContext context, KubernetesConfigDataReso
ConfigurableBootstrapContext bootstrapContext = context.getBootstrapContext();
Environment env = resource.getEnvironment();

if (bootstrapContext.isRegistered(ConfigMapPropertySourceLocator.class)) {
Copy link
Contributor Author

@wind57 wind57 May 12, 2023

Choose a reason for hiding this comment

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

this is an interesting change right here. For config data, we need to provide the sources in the order we want, looks like @Order is not respected. The documentation of ConfigData constructor is a bit interesting:

propertySources - the config data property sources in ascending priority order.

I assume it means we need to provide the proper order to it, but it accepts a Collection as input; which can be a Set, for example... which has no order. I would have expected a List here; but then again a LinkedHashSet is ordered too, tricky!

Sequenced collections would be a great addition to these kind of APIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only important change, everything else are just tests. I had to adjust them a little, cause in the main branch - we have two types of tests : bootstrap and config data. Functionally, the tests have not changed at all.

@wind57 wind57 marked this pull request as ready for review May 12, 2023 11:42
@wind57
Copy link
Contributor Author

wind57 commented May 12, 2023

@ryanjbaxter ready. fixes the current build + add some config data tests for the issues we did yesterday on the 2.1.x branch

@ryanjbaxter ryanjbaxter added this to the 3.0.3 milestone May 12, 2023
@ryanjbaxter ryanjbaxter merged commit bf38b57 into spring-cloud:main May 12, 2023
@ryanjbaxter
Copy link
Contributor

Thanks!

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.

2 participants