-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Move http cache writing to cacheitem. #4919
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
9 Ignored Deployments
|
Someone is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
d384f5e
to
a220a80
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewer's guide!
@@ -31,7 +31,7 @@ type CacheItem struct { | |||
tw *tar.Writer | |||
zw io.WriteCloser | |||
fileBuffer *bufio.Writer | |||
handle io.Reader | |||
handle interface{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caches are Reader | Writer | Closer
but you can't express that in Go. So this goes to interface{}
and each place that needs it does a type assertion.
#4634 made this partially generic, this finishes the work.
// log.Printf("caching file %v", file) | ||
if err := cache.storeFile(tw, file); err != nil { | ||
log.Printf("[ERROR] Error uploading artifact %s to HTTP cache due to: %s", file, err) | ||
// TODO(jaredpalmer): How can we cancel the request at this point? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fixed by using the channel. Previously failing here didn't actually blow up the artifact.
} | ||
} | ||
} | ||
func (cache *httpCache) write(w io.WriteCloser, anchor turbopath.AbsoluteSystemPath, files []turbopath.AnchoredSystemPath, cacheErrorChan chan error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now 1:1 with cache_fs
's implementation.
ffb93b6
to
c82a628
Compare
528d6fd
to
b1f9352
Compare
b1f9352
to
d90427e
Compare
The CI test run that confirms cross-platform compat for cacheitem is here: https://github.com/nathanhammond/turbo/actions/runs/5002277231 |
The HTTP cache creates its own tar files that aren't
cacheitem
. This migrates them to usecacheitem
, same as filesystem cache.