Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support multiple labels in NativeEnvironmentRepository #2454

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

ryanjbaxter
Copy link
Contributor

Related to #2449

@ryanjbaxter ryanjbaxter added this to the 4.1.4 milestone Aug 23, 2024
@spencergibb
Copy link
Member

Maybe for main?

@kvmw
Copy link
Contributor

kvmw commented Sep 25, 2024

@spencergibb

Is the change in this PR is just for this particular repository or it supposed to be implemented by all other repositories? I am thinking about JGitEnvironmentRepository and other similar repos. I don't think supporting multiple labels would be easy for JGitEnvironmentRepository
.

@ryanjbaxter
Copy link
Contributor Author

I am curious why you think it would not be easy for JGitEnvironmentRepository?

@ryanjbaxter
Copy link
Contributor Author

I guess I see why now since a label corresponds to a branch and we can only checkout and use a single branch at a time

@kvmw
Copy link
Contributor

kvmw commented Sep 25, 2024

yes, And It uses NativeEnvironmentRepository via AbstractScmEnvironmentRepository internally.

So I supposed we need to split the label and for each entry we have to run what is currently in AbstractScmEnvironmentRepository::findOne.

Something like:

var labels = label.split(",");
for (String lbl : labels) {
                NativeEnvironmentRepository delegate = new NativeEnvironmentRepository(getEnvironment(),
				new NativeEnvironmentProperties(), this.observationRegistry);
		Locations locations = getLocations(application, profile, lbl);
		delegate.setSearchLocations(locations.getLocations());
		Environment result = delegate.findOne(application, profile, "", includeOrigin);
		result.setVersion(locations.getVersion());
		result.setLabel(lbl);
}

I am just worried about the impact of such a change. Maybe I am just being oversuspicious.

@ryanjbaxter ryanjbaxter removed the bug label Oct 2, 2024
@ryanjbaxter ryanjbaxter modified the milestones: 4.1.4, 4.2.0-M2 Oct 2, 2024
@ryanjbaxter ryanjbaxter changed the base branch from 4.1.x to main October 2, 2024 14:12
@ryanjbaxter ryanjbaxter linked an issue Oct 2, 2024 that may be closed by this pull request
@ryanjbaxter ryanjbaxter merged commit e48ac40 into spring-cloud:main Oct 2, 2024
1 check passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Support Multiple Labels In Environment Repositories
3 participants