cache: add SetSnapshot and GetSnapshot to LinearCache #437
Conversation
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
|
Sorry about the force push 🙏 I started the work from |
|
Thanks for opening this! I'll review |
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
|
hey, @alecholmez do you have any more suggestions? |
|
@jakubdyszkiewicz I think this is good from my end, can you address the merge conflicts? @snowp you have any thoughts? |
| cache.version += 1 | ||
|
|
||
| modified := map[string]struct{}{} | ||
| for name := range cache.resources { |
There was a problem hiding this comment.
Some comments would be make this easier to follow, like // First account for removed resources. and a similar one for the other for loop
pkg/cache/v3/linear.go
Outdated
| cache.resources = resources | ||
| for name := range resources { | ||
| // We assume all resources passed to SetResources are changed. | ||
| // Otherwise we would have to do proto.Equal on resources which is pretty expensive operation |
There was a problem hiding this comment.
There are other solutions to this (like per version resource values like how Delta is modeled), but it's probably fine to leave it as this for now.
…t-snapshot Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
|
@snowp I fixed comments and conflicts |
|
@alecholmez for merge |
There was a problem hiding this comment.
This looks good @jakubdyszkiewicz, thanks for the contribution!
The implementation discussed in #433
SetSnapshotandGetSnapshotenables users to useLinearCachein a more convenient way if the major use case is sendingDiscoveryRequestwith emptyresourceNames.