You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
we can handle this quite elegantly without the resource discriminators, is there are use case (also for external resources) when we need this former approach anymore? If we cannot think we should consider removing this for v5
The only case with is not handled now elegantly is the read-only dependent resource since that does not have the desired state by default. There are multiple choices regarding this:
the users will have still to provide the desired for read-only but just with the metadata only. Maybe we can make this nicer, providing an API to override that just returns a ResourceID.
we keep the discriminators
the read only is resource is still a corner case, to by just overriding the DependentResource.getSecondaryResource the user can provide it's own implementation.
I think we should keep them for optimization purposes as well. If the desired state is costly to compute and there are better ways to decided which secondary resource needs to be used, I think it still makes sense to use a discriminator even if it shouldn't be needed in most cases. That said, we could indeed tell people to override getSecondaryResource in that situation.
indeed tell people to override getSecondaryResource in that situation
exactly, basically resource discriminator puts nothing on table now, as we discussed before the only reason to create was to have a class representing the concept explicitly, so it is settable explicitly on a resource. But it's API is basically just the context:
Optional<R> distinguish(Class<R> resource, P primary, Context<P> context);
Yes but if it's costly to compute, we should try to avoid computing it several times… maybe we should record the computed desired state(s) in the Context so that they are available for downstream usage once they have been computed once?
After using JOSDK for quite some time now, I think we only need discriminators for dependents because dependents currently have no identifiers. This is why @csviri proposes to evaluate the resource that the desired method returns.
To overcome this problem of missing identifier, I've added the following method to all of my dependent classes public static String getName(P primary) where P is the primary CRD type. This allows me to derive the name from the name of the primary in a consistent way throughout my code, e.g. I call getName in every discriminator and I am able to find the Kubernetes via fabric8 client even without the context (e.g. useful for test cases).
You could force users to define the identifier upfront (e.g. by an abstract method ResourceID getId(P primary, Context<P> context) that has to be implemented). This has several benefits:
You would have a way to determine the resource id even without calling desired
You would not need discriminators because you can rely on the getId method to distinguish resources of the same type
You could even enforce that the resource returned by desired always has the name/namespace returned by getId (today it is possible that I change the name of the resource between multiple calls of the desired method)
Activity
csviri commentedon Feb 28, 2024
The only case with is not handled now elegantly is the read-only dependent resource since that does not have the desired state by default. There are multiple choices regarding this:
ResourceID.DependentResource.getSecondaryResourcethe user can provide it's own implementation.metacosm commentedon Feb 28, 2024
I think we should keep them for optimization purposes as well. If the desired state is costly to compute and there are better ways to decided which secondary resource needs to be used, I think it still makes sense to use a discriminator even if it shouldn't be needed in most cases. That said, we could indeed tell people to override
getSecondaryResourcein that situation.csviri commentedon Feb 28, 2024
exactly, basically resource discriminator puts nothing on table now, as we discussed before the only reason to create was to have a class representing the concept explicitly, so it is settable explicitly on a resource. But it's API is basically just the
context:Optional<R> distinguish(Class<R> resource, P primary, Context<P> context);On the other hand desired needs to be always computed, except in the read-only (what is already kinda exotic case https://javaoperatorsdk.io/docs/dependent-resources#read-only-dependent-resources-vs-event-source )
But will examine the problem more in depth.
metacosm commentedon Feb 28, 2024
Yes but if it's costly to compute, we should try to avoid computing it several times… maybe we should record the computed desired state(s) in the Context so that they are available for downstream usage once they have been computed once?
csviri commentedon Feb 28, 2024
yes definitelly, that is something will be done even if the discriminators are not removed.
Javatar81 commentedon Feb 28, 2024
After using JOSDK for quite some time now, I think we only need discriminators for dependents because dependents currently have no identifiers. This is why @csviri proposes to evaluate the resource that the desired method returns.
To overcome this problem of missing identifier, I've added the following method to all of my dependent classes
public static String getName(P primary)where P is the primary CRD type. This allows me to derive the name from the name of the primary in a consistent way throughout my code, e.g. I callgetNamein every discriminator and I am able to find the Kubernetes via fabric8 client even without the context (e.g. useful for test cases).You could force users to define the identifier upfront (e.g. by an abstract method
ResourceID getId(P primary, Context<P> context)that has to be implemented). This has several benefits: