fixes and api improvements #775
Conversation
| @@ -6,5 +6,9 @@ | |||
|
|
|||
| Optional<RetryInfo> getRetryInfo(); | |||
|
|
|||
| <T> T getSecondaryResource(Class<T> expectedType, String... qualifier); | |||
| default <T> T getSecondaryResource(Class<T> expectedType) { | |||
There was a problem hiding this comment.
@metacosm is this ok with you, kind would like it to limit it in this scope. Since more qualifiers does not make sense, and if dependent resources will use the previous way it will be backwards compatible anyway. So it can be changed back without breaking the api.
| EventSourceManager manager = initManager(); | ||
|
|
||
| CachingEventSource eventSource = mock(CachingEventSource.class); | ||
| when(eventSource.getResourceClass()).thenReturn(String.class); | ||
| when(eventSource.getResourceClass()).thenReturn(InformerEventSource.class); |
There was a problem hiding this comment.
That's not correct. getResourceClass is supposed to return the class handled by the event source. So if you specify that the resource class is InformerEventSource, this means that your event source deals with InformerEventSources which doesn't really make sense. Granted, String doesn't make a lot more sense either but it's a little bit more realistic and doesn't risk clashing with the EventSource class itself (which should be reserved for cases where there is no associated type with an EventSource).
There was a problem hiding this comment.
Ohh, yes, sorry my fault, will change it!
| import io.fabric8.kubernetes.model.annotation.Version; | ||
|
|
||
| @Group("sample.javaoperatorsdk.io") | ||
| @Version("v1") | ||
| @Kind("TestCustomResource") |
There was a problem hiding this comment.
This is needed to compare resources when we register a new reonciler. There was a tests for the registration of the same resource with multiple versions - what should fail. It did not fail without this, could not determine the kind. It was not failing until now because there were junit4 annotations used on test class
There was a problem hiding this comment.
see ControllerManagerTest.java changes
There was a problem hiding this comment.
If that functionality should work without this annotation, that should be a separate issues IMHO.
There was a problem hiding this comment.
It shouldn't be needed here. Just for the v2.
| import io.fabric8.kubernetes.model.annotation.Version; | ||
|
|
||
| @Group("sample.javaoperatorsdk.io") | ||
| @Version("v2") | ||
| @Kind("TestCustomResource") |
There was a problem hiding this comment.
OK, I guess you added it to the V1 version for symmetry?
No description provided.