Skip to content

Conversation

@metacosm
Copy link
Collaborator

@metacosm metacosm commented Jan 29, 2026

Alternative to #3130, to discuss

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 29, 2026
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 29, 2026
@metacosm metacosm changed the base branch from main to next January 29, 2026 22:10
@metacosm metacosm requested review from csviri and xstefank and removed request for xstefank January 29, 2026 22:12
@metacosm metacosm self-assigned this Jan 29, 2026
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 29, 2026
Copy link
Collaborator

@csviri csviri left a comment

Choose a reason for hiding this comment

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

Made some minor comments, pls add unit tests! Otherwise looks great, thank you!!


default <R> Stream<R> getSecondaryResourcesAsStream(Class<R> expectedType) {
return getSecondaryResources(expectedType).stream();
return getSecondaryResourcesAsStream(expectedType, false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here we could use the former implementation and call this in case in the new method the deduplication is false.

@metacosm metacosm force-pushed the latest-distinct-alternative branch from fdfaa63 to 3c9de5c Compare February 2, 2026 13:31
@metacosm metacosm marked this pull request as ready for review February 2, 2026 13:32
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 2, 2026
@openshift-ci openshift-ci bot requested review from csviri and xstefank February 2, 2026 13:32
}
return stream
.collect(
Collectors.toUnmodifiableMap(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the end, I couldn't make the implementation work without collecting to Map, I iterated over the code and ended collecting manually…

@metacosm metacosm force-pushed the latest-distinct-alternative branch 4 times, most recently from afa2dc8 to 7158917 Compare February 2, 2026 17:33
Copy link
Collaborator

@csviri csviri left a comment

Choose a reason for hiding this comment

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

Added some nits, pls make the CI happy, otherwise LGTM!


/**
* Whether this ResourceID points to the same resource as the one identified by the specified name
* and namespace. Note that this doesn't take API version or Kind into account so this should only
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: does not take Group either.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, didn't want to get into all the details 😅
Actually planning on adding this to HasMetadata in the client.

var updatedResource =
extension.get(LatestDistinctTestResource.class, TEST_RESOURCE_NAME);
assertThat(updatedResource.getStatus()).isNotNull();
// Should see 3 distinct ConfigMaps
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment needs ro be adjusted

@metacosm metacosm force-pushed the latest-distinct-alternative branch from 7158917 to a128bd7 Compare February 3, 2026 10:19
@metacosm metacosm merged commit b226145 into next Feb 3, 2026
25 checks passed
@metacosm metacosm deleted the latest-distinct-alternative branch February 3, 2026 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support distinct if multiple ES for same type for getSecondaryResources()

5 participants