Skip to content
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

Refine the Informer Store interface #3091

Closed
shawkins opened this issue May 8, 2021 · 3 comments
Closed

Refine the Informer Store interface #3091

shawkins opened this issue May 8, 2021 · 3 comments
Assignees
Milestone

Comments

@shawkins
Copy link
Contributor

shawkins commented May 8, 2021

There are several options to improve the Store interface.

The most aggressive, which would deviate from the go client, would be to remove all of the mutative / internal methods to a separate interface - add, update, delete, replace, resync, isPopulated, hasSynced would be part of a - which would then no longer be directly exposed via the Indexer interface. The rationale is that these either have no-op behavior or should not be called directly.

A lesser refactoring would simply align the interface with the go client, such that newer methods added in fabric8 are moved to a separate interface.

Once #3061 is committed all of the cache methods can be type-safe - no need to reference the object type.

#3090 proposes removing the isPopulated method altogether.

@shawkins
Copy link
Contributor Author

@rohanKanojia a full proposal for refactoring after #3108 looks like shawkins#3

@rohanKanojia
Copy link
Member

@shawkins : Thanks, let me review it later today 👍

@shawkins shawkins self-assigned this May 20, 2021
shawkins added a commit to shawkins/kubernetes-client that referenced this issue May 24, 2021
…sSynced

splitting internal and mutative methods to SyncableStore

removing isPopulated and directly wiring the cache to be used
this adds an isRunning method to the SharedInformer.  this clarifies the
cache methods to be more like a map and the underlying map is now
concurrent to remove read locks
shawkins added a commit to shawkins/kubernetes-client that referenced this issue May 25, 2021
…sSynced

splitting internal and mutative methods to SyncableStore

removing isPopulated and directly wiring the cache to be used
this adds an isRunning method to the SharedInformer.  this clarifies the
cache methods to be more like a map and the underlying map is now
concurrent to remove read locks
shawkins added a commit to shawkins/kubernetes-client that referenced this issue May 25, 2021
…sSynced

splitting internal and mutative methods to SyncableStore

removing isPopulated and directly wiring the cache to be used
this adds an isRunning method to the SharedInformer.  this clarifies the
cache methods to be more like a map and the underlying map is now
concurrent to remove read locks
manusa pushed a commit that referenced this issue Jun 9, 2021
splitting internal and mutative methods to SyncableStore

removing isPopulated and directly wiring the cache to be used
this adds an isRunning method to the SharedInformer.  this clarifies the
cache methods to be more like a map and the underlying map is now
concurrent to remove read locks
@shawkins
Copy link
Contributor Author

shawkins commented Jun 9, 2021

Closed via #3167

@shawkins shawkins closed this as completed Jun 9, 2021
@manusa manusa added this to the 5.5.0 milestone Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants