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.

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

refactor the cache.Cache interface #161

Closed
mostynb opened this issue Jan 16, 2020 · 2 comments · Fixed by #162
Closed

refactor the cache.Cache interface #161

mostynb opened this issue Jan 16, 2020 · 2 comments · Fixed by #162
Assignees
@mostynb

Comments

@mostynb
Copy link
Collaborator

mostynb commented Jan 16, 2020

I think we should refactor the cache.Cache interface. At the moment we have a single cache.Cache interface that is used both by the disk backend and the proxy backends (s3, http/gcs). The proxy backends contain the same mostly copy+pasted logic for using the local/disk backend, and just forward some calls (like Stats) directly to the local/disk backend.

Instead, we could move the duplicated logic into the disk backend, and have the disk backend make calls to an optional proxy backend. The proxy backends would then implement a smaller interface, and their code would be simpler. This in turn would make it simpler to implement action cache data integrity checks in one place (the disk backend).
@mostynb mostynb self-assigned this Jan 16, 2020
@mostynb
Copy link
Collaborator Author

mostynb commented Jan 16, 2020

@buchgr: I think it makes sense to do this refactoring before attempting the action cache/sync changes.

@mostynb mostynb changed the title refactor cache.Cache interface refactor the cache.Cache interface Jan 16, 2020
@buchgr
Copy link
Owner

buchgr commented Jan 16, 2020

Sounds about right.

mostynb added a commit to mostynb/bazel-remote that referenced this issue Jan 16, 2020
@mostynb
refactor the cache.Cache interface
369ddc3 
Since all the proxy backends use the same copy+pasted logic to use
the disk cache, refactor to put that functionality in the disk cache.
This simplifies the proxy backends.

Fixes buchgr#161.
@mostynb mostynb mentioned this issue Jan 16, 2020
Merged
mostynb added a commit to mostynb/bazel-remote that referenced this issue Jan 20, 2020
@mostynb
refactor the cache.Cache interface
32272d2 
Since all the proxy backends use the same copy+pasted logic to use
the disk cache, refactor to put that functionality in the disk cache.
This simplifies the proxy backends.

Contains is now also proxied to the backend, instead of only
returning whether or not the local disk cache contains the item.

Fixes buchgr#161.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Jan 20, 2020
@mostynb
refactor the cache.Cache interface
1788bfe 
Since all the proxy backends use the same copy+pasted logic to use
the disk cache, refactor to put that functionality in the disk cache.
This simplifies the proxy backends.

Contains is now also proxied to the backend, instead of only
returning whether or not the local disk cache contains the item.

Fixes buchgr#161.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Jan 20, 2020
@mostynb
refactor the cache.Cache interface
54dd298 
Since all the proxy backends use the same copy+pasted logic to use
the disk cache, refactor to put that functionality in the disk cache.
This simplifies the proxy backends.

Contains is now also proxied to the backend, instead of only
returning whether or not the local disk cache contains the item.

Fixes buchgr#161.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Jan 20, 2020
@mostynb
refactor the cache.Cache interface
e7fc03b 
Since all the proxy backends use the same copy+pasted logic to use
the disk cache, refactor to put that functionality in the disk cache.
This simplifies the proxy backends.

Contains is now also proxied to the backend, instead of only
returning whether or not the local disk cache contains the item.

Fixes buchgr#161.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Jan 24, 2020
@mostynb
refactor the cache.Cache interface
7702c5f 
Since all the proxy backends use the same copy+pasted logic to use
the disk cache, refactor to put that functionality in the disk cache.
This simplifies the proxy backends.

Contains is now also proxied to the backend, instead of only
returning whether or not the local disk cache contains the item.

Fixes buchgr#161.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Jan 24, 2020
@mostynb
refactor the cache.Cache interface
c3b9160 
Since all the proxy backends use the same copy+pasted logic to use
the disk cache, refactor to put that functionality in the disk cache.
This simplifies the proxy backends.

Contains is now also proxied to the backend, instead of only
returning whether or not the local disk cache contains the item.

Fixes buchgr#161.
mostynb added a commit that referenced this issue Jan 24, 2020
@mostynb
refactor the cache.Cache interface
3f62926 
Since all the proxy backends use the same copy+pasted logic to use
the disk cache, refactor to put that functionality in the disk cache.
This simplifies the proxy backends.

Contains is now also proxied to the backend, instead of only
returning whether or not the local disk cache contains the item.

Fixes #161.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mostynb mostynb

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor the cache.Cache interface mostynb/bazel-remote
2 participants
@buchgr @mostynb