Skip to content

feat: make primary cache accessible from Context #2053

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 14, 2023
Merged

feat: make primary cache accessible from Context #2053

merged 3 commits into from
Sep 14, 2023

Conversation

metacosm
Copy link
Collaborator

@metacosm metacosm commented Sep 12, 2023 •
edited
Loading

Alternative to #2051

@metacosm metacosm self-assigned this Sep 12, 2023
@metacosm metacosm requested a review from csviri September 12, 2023 08:23
@openshift-ci openshift-ci bot requested a review from adam-sandor September 12, 2023 08:23
metacosm
metacosm commented Sep 12, 2023
View reviewed changes
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java Outdated
* @return the {@link IndexerResourceCache} associated with the associated {@link Reconciler} for
* this context
*/
IndexerResourceCache<P> getPrimaryCache();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the fact that the IndexerResourceCache interface allows access to creating new indices at any time when these would actually only take effect whenever the associated informers are initialized so I think we should split this interface into a read-only part and one to manage indexers. The alternative would be to only expose ResourceCache instead of IndexerResourceCache.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added an IndexedResourceCache interface that extends ResourceCache to add the indexed-based retrieval and moved both classes to the reconciler package since they'd now be part of the API… It shouldn't break anything since the only thing that was used externally was the IndexerResourceCache interface.

@metacosm metacosm force-pushed the access-primary-resources branch from 2f2171d to f3c7b39 Compare September 12, 2023 08:53
@csviri
Copy link
Collaborator

csviri commented Sep 12, 2023

This should rather target next imo, since it's a feature.

@csviri
Copy link
Collaborator

csviri commented Sep 12, 2023

But I think we should we can go this way, just exposing the cache, and see if anybody would need actually more.

csviri
csviri approved these changes Sep 12, 2023
View reviewed changes
Copy link
Collaborator

@csviri csviri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, apart that should target next.

@csviri csviri mentioned this pull request Sep 12, 2023
Closed
@metacosm metacosm changed the base branch from main to next September 12, 2023 12:06
@metacosm
feat: make primary cache accessible from Context
e2928bb 
Signed-off-by: Chris Laprun <claprun@redhat.com>
@metacosm
refactor: introduce IndexedResourceCache interface
216582c 
Signed-off-by: Chris Laprun <claprun@redhat.com>
@metacosm
refactor: move read-only cache interfaces to api package
08af84f 
Signed-off-by: Chris Laprun <claprun@redhat.com>
@metacosm metacosm force-pushed the access-primary-resources branch from f3c7b39 to 08af84f Compare September 13, 2023 06:38
@metacosm metacosm requested a review from csviri September 13, 2023 06:42
@metacosm metacosm merged commit 307dcc8 into next Sep 14, 2023
@metacosm metacosm deleted the access-primary-resources branch September 14, 2023 14:07
metacosm added a commit that referenced this pull request Sep 15, 2023
@metacosm
feat: make primary cache accessible from Context (#2053)
5047ca0 
* feat: make primary cache accessible from Context

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: introduce IndexedResourceCache interface

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: move read-only cache interfaces to api package

Signed-off-by: Chris Laprun <claprun@redhat.com>

---------

Signed-off-by: Chris Laprun <claprun@redhat.com>
csviri pushed a commit that referenced this pull request Sep 18, 2023
@metacosm @csviri
feat: make primary cache accessible from Context (#2053)
d309cba 
* feat: make primary cache accessible from Context

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: introduce IndexedResourceCache interface

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: move read-only cache interfaces to api package

Signed-off-by: Chris Laprun <claprun@redhat.com>

---------

Signed-off-by: Chris Laprun <claprun@redhat.com>
csviri pushed a commit that referenced this pull request Sep 18, 2023
@metacosm @csviri
feat: make primary cache accessible from Context (#2053)
0f8c82e 
* feat: make primary cache accessible from Context

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: introduce IndexedResourceCache interface

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: move read-only cache interfaces to api package

Signed-off-by: Chris Laprun <claprun@redhat.com>

---------

Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
csviri pushed a commit that referenced this pull request Oct 3, 2023
@metacosm @csviri
feat: make primary cache accessible from Context (#2053)
88dda4c 
* feat: make primary cache accessible from Context

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: introduce IndexedResourceCache interface

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: move read-only cache interfaces to api package

Signed-off-by: Chris Laprun <claprun@redhat.com>

---------

Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
shawkins pushed a commit to shawkins/java-operator-sdk that referenced this pull request Oct 4, 2023
@metacosm @shawkins
feat: make primary cache accessible from Context (operator-framework#…
c0bb8f2 
…2053)

* feat: make primary cache accessible from Context

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: introduce IndexedResourceCache interface

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: move read-only cache interfaces to api package

Signed-off-by: Chris Laprun <claprun@redhat.com>

---------

Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
csviri pushed a commit that referenced this pull request Oct 4, 2023
@metacosm @csviri
feat: make primary cache accessible from Context (#2053)
7442605 
* feat: make primary cache accessible from Context

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: introduce IndexedResourceCache interface

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: move read-only cache interfaces to api package

Signed-off-by: Chris Laprun <claprun@redhat.com>

---------

Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
csviri pushed a commit that referenced this pull request Oct 18, 2023
@metacosm @csviri
feat: make primary cache accessible from Context (#2053)
21121da 
* feat: make primary cache accessible from Context

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: introduce IndexedResourceCache interface

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: move read-only cache interfaces to api package

Signed-off-by: Chris Laprun <claprun@redhat.com>

---------

Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@csviri csviri csviri approved these changes

@adam-sandor adam-sandor Awaiting requested review from adam-sandor

Assignees

@metacosm metacosm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@metacosm @csviri