Skip to content

[cachetools] add NewUploadWriter and tests #9406

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

Merged
merged 4 commits into from
May 22, 2025
Merged

[cachetools] add NewUploadWriter and tests #9406

merged 4 commits into from
May 22, 2025

Conversation

dan-stowell
Copy link
Contributor

@dan-stowell dan-stowell commented May 20, 2025

This change introduces NewUploadWriter and tests for UploadWriter. It leaves the UploadFromReader path untouched, since my first need for UploadWriter is as a best-effort writer for caching OCI image layers. A later change will have UploadFromReader use UploadWriter.

@dan-stowell dan-stowell reopened this May 21, 2025
@dan-stowell dan-stowell marked this pull request as ready for review May 21, 2025 15:23
@dan-stowell dan-stowell changed the title [cachetools] add NewUploadWriter, use in UploadFromReader [cachetools] add NewUploadWriter and tests May 21, 2025
if uw.committed {
return nil
}
if uw.sendErr != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sendErr could be EOF, right? In that case, do you still want to CloseAndRecv so you can get the committed size?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • In Commit(), CloseAndRecv() is still called even if flush(..) returns EOF.
  • In Write(...), I opted to return ErrShortWrite instead of EOF. Tests do not seem to be hitting it.
  • Added test cases for the UploadWriter paths.

@dan-stowell dan-stowell merged commit 162efa3 into master May 22, 2025
15 checks passed
@dan-stowell dan-stowell deleted the dan-uw branch May 22, 2025 15:18
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

Successfully merging this pull request may close these issues.

2 participants