feat: provide de-duplicated secondary resources stream on Context #3141
Conversation
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
…tor/api/reconciler/ReconcileUtils.java Co-authored-by: Martin Stefanko <xstefank122@gmail.com>
Signed-off-by: Chris Laprun <metacosm@gmail.com>
openshift-ci
bot
added
the
do-not-merge/work-in-progress
openshift-merge-robot
added
the
needs-rebase
metacosm
requested review from
csviri and
xstefank
and removed request for
xstefank
Alternative without requiring collecting to a Map Signed-off-by: Chris Laprun <metacosm@gmail.com>
openshift-merge-robot
removed
the
needs-rebase
...-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java
Show resolved
Hide resolved
...-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java
Show resolved
Hide resolved
| * @param <T> the type of HasMetadata objects | ||
| * @return a collector that produces a collection of deduplicated Kubernetes objects | ||
| */ | ||
| public static <T extends HasMetadata> Collector<T, ?, Collection<T>> latestDistinct() { |
There was a problem hiding this comment.
wasn't this moved to ReconcileUtilsInternal
There was a problem hiding this comment.
If we were to keep this implementation, this actually would be removed
csviri
left a comment
csviri
left a comment
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
Here we could use the former implementation and call this in case in the new method the deduplication is false.
| } | ||
|
|
||
| final var idToLatest = deduplicate ? new HashMap<ResourceID, String>() : null; | ||
| final var stream = |
There was a problem hiding this comment.
As mentioned above for sake of simplicity/readbaility can if deduplicate is false could we just use the former implementation?
There was a problem hiding this comment.
We could but this implementation avoids creating intermediary Streams, though I agree that it's slightly less readable. Let me see what I can do about that.
| return controller.getEventSourceManager().getEventSourcesFor(expectedType).stream() | ||
| .map(es -> es.getSecondaryResources(primaryResource)) | ||
| .flatMap(Set::stream); | ||
| public <R> Stream<R> getSecondaryResourcesAsStream(Class<R> expectedType, boolean deduplicate) { |
There was a problem hiding this comment.
Please add comprehensive javadoc
Alternative to #3130, to discuss