-
Notifications
You must be signed in to change notification settings - Fork 194
Modified approach to Fix Zero Byte Uploads #1860
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
base: main
Are you sure you want to change the base?
Modified approach to Fix Zero Byte Uploads #1860
Conversation
d25131d
to
87bacf0
Compare
Maybe we can do a simple upload at the beginning for a zero byte uploads. The problem appears to be that the loop breaks without initializing the upload session. So, a simple zero-byte upload in the beginning should solve this issue. |
|
||
Ok(()) | ||
} | ||
// I'll delete this but it did not actually test and prevent the issue we were addressing. |
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.
I'm assuming this is working as expected right?
The problem appears to be during zero byte uploads. The get operation should work fine in this case.
Let me test this out locally as well.
87bacf0
to
5b0cbd5
Compare
.err_tip(|| "Could not convert max_retry_buffer_size to u64")?, | ||
); | ||
|
||
// Special case for known zero-byte uploads: Start resumable and finalize immediately. |
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.
Why do a resumable upload if the exact size is zero. Wouldn't it be better to do a simple upload for a zero byte uploads.
Description
@chrisstaite reported issues with the zero bytes and the GCS store. I'm trying to understand why this wasn't resolved bya. previous test to address it.
Fixes # (issue)
Type of change
How Has This Been Tested?
I wrote a test. It needs to be tested on a live GCP instance. I'm currently on a plane with limited bandwidth.
Checklist
bazel test //...
passes locallygit amend
see some docsThis change is