feat: remove primary to secondary mapper (handled automatically) #1161
Conversation
|
@metacosm I would actually put this into 3.0 since (if does not make trouble with quarkus extension), although its quite last minute, but simplifies usage a lot. |
| @@ -28,9 +32,28 @@ public Optional<RetryInfo> getRetryInfo() { | |||
| return Optional.ofNullable(retryInfo); | |||
| } | |||
|
|
|||
| @Override | |||
| @SuppressWarnings("unchecked") | |||
| public <T> List<T> getSecondaryResources(Class<T> expectedType) { | |||
There was a problem hiding this comment.
Shouldn't this be a Set instead of List, actually? Don't we want to return unique resources? Also, there's no ordering there so a List is not required.
There was a problem hiding this comment.
Fine with that will change.
| import io.javaoperatorsdk.operator.processing.event.ResourceID; | ||
| import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper; | ||
|
|
||
| public class PrimaryToSecondaryIndex<R extends HasMetadata> { |
There was a problem hiding this comment.
This class should probably be package-private, no?
There was a problem hiding this comment.
No, it is also used by the user in case to define custom mapping. (We have default by owner reference, but that does not work for example outside of namespace. See also support in KubernetesDependentResource.
There was a problem hiding this comment.
I don't understand what you mean. This class is only used by InformerEventSource internally and since it's the class implementing the automated indexing of secondary resources, it shouldn't be exposed anyway.
There was a problem hiding this comment.
Sorry, I though this is the mapper. Yes this can be package private, will change it.
| if (log.isDebugEnabled()) { | ||
| log.debug("On delete event received for resource id: {}", ResourceID.fromResource(resource)); | ||
| } | ||
| primaryToSecondaryIndex.onAddOrUpdate(resource); |
There was a problem hiding this comment.
Shouldn't that be onDelete instead?
There was a problem hiding this comment.
uhh, yes, fixed thx.
| } | ||
| super.onDelete(resource, b); | ||
| propagateEvent(resource); | ||
| private boolean temporalCacheHasResourceWithVersionAs(R resource) { |
There was a problem hiding this comment.
| private boolean temporalCacheHasResourceWithVersionAs(R resource) { | |
| private boolean temporaryCacheHasResourceWithSameVersionAs(R resource) { |
There was a problem hiding this comment.
changed this separatelly in a commit, because here might break the build.
|
Kudos, SonarCloud Quality Gate passed! |
No description provided.