-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fileserver: Read Etags from precomputed files #6222
Conversation
01a0959
to
77a845f
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.
Thanks for the great enhancement! This looks pretty good but I just have a few comments.
77a845f
to
2bd54ef
Compare
Thanks for taking a look at this Matt. All the feedback discussions should now be resolved. |
_, err := fs.Stat(fileSystem, etagFilename) | ||
if err != nil { | ||
continue | ||
} |
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.
Is the purpose of this to check whether we can access the file? Maybe we can eliminate this system call and instead just call fs.ReadFile()
directly. Then:
if errors.Is(err, fs.ErrNotExist) {
continue
}
if err != nil {
return "", fmt.Errorf("cannot read etag from file %s: %v", etagFilename, err)
}
What do you think?
(I just noticed this. sorry. lol)
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.
Much much better. Just changed it. Thanks!
2bd54ef
to
6fc1713
Compare
Awesome, thanks for working on this! |
Initial pass at closing #5734