Skip to content

Consider deprecating getSecondaryResource(Class,name) method from Context #1240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
csviri opened this issue May 25, 2022 · 0 comments · Fixed by #1378
Closed

Consider deprecating getSecondaryResource(Class,name) method from Context #1240

csviri opened this issue May 25, 2022 · 0 comments · Fixed by #1378
Assignees
Labels
api-changes-epic needs-discussion Issue needs to be discussed more before working on it
Milestone

Comments

@csviri
Copy link
Collaborator

csviri commented May 25, 2022

(And removing in next major release.)

https://github.com/java-operator-sdk/java-operator-sdk/blob/58d2ea32cad4144aa63e3836e616a23c2fa911f0/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java#L19-L21

Pros:

  1. we anyway have now <T> Set<T> getSecondaryResources(Class<T> expectedType); which return all the resource from all the related event sources of that type.
  2. We should move users to use just one event source per type, since it's more efficient.
  3. The event sources and resource cached are now tightly coupled, but this is not necessarily a good thing, might event change in the future. So ideally this interface should not know about the event sources and their names (what we might not even need then in the future)
  4. We could remove the quite complex code related to this logic:

https://github.com/java-operator-sdk/java-operator-sdk/blob/58d2ea32cad4144aa63e3836e616a23c2fa911f0/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java#L100-L140

Const:

  1. this still might be useful to distinguish or make it easier to find a cached resource.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes-epic needs-discussion Issue needs to be discussed more before working on it
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant