Fix FilesExt upload fails when content size is zero #1088
Conversation
yuanjieding-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
yuanjieding-db
force-pushed
the
yuanjie/fix-zero-size-file-upload
branch
from
d9e6e97 to
3f1cea1
Compare
yuanjieding-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
yuanjieding-db
force-pushed
the
yuanjie/fix-zero-size-file-upload
branch
from
3f1cea1 to
efb8b02
Compare
yuanjieding-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
yuanjieding-db
force-pushed
the
yuanjie/fix-zero-size-file-upload
branch
from
efb8b02 to
bf741e8
Compare
yuanjieding-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
yuanjieding-db
force-pushed
the
yuanjie/fix-zero-size-file-upload
branch
from
bf741e8 to
c6c2892
Compare
yuanjieding-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
|
jenkins trigger all |
| parallelism=parallelism, | ||
| ) | ||
| if ctx.use_parallel: | ||
| if ctx.use_parallel and ctx.content_length >= self._config.files_ext_multipart_upload_min_stream_size: |
There was a problem hiding this comment.
Can content_length be none here?
There was a problem hiding this comment.
No, in the upload_from function, content_length will always be set using os.path.getsize(source_path), which only returns a number.
There was a problem hiding this comment.
Maybe, not in this PR. Let's avoid using the same uploadContext in both methods, as they may differ for different functions.
FWIW, I am planning to add a static type checker in the Python SDK, which will block such scenarios.
tests/test_files.py
Outdated
| ), | ||
| MultipartUploadTestCase( | ||
| "Multipart upload successful: empty file or empty seekable stream", | ||
| content_size=0, # less than part size |
There was a problem hiding this comment.
| content_size=0, # less than part size | |
| content_size=0, # content with zero length |
yuanjieding-db
force-pushed
the
yuanjie/fix-zero-size-file-upload
branch
from
c6c2892 to
33e4895
Compare
yuanjieding-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
yuanjieding-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
| parallelism=parallelism, | ||
| ) | ||
| if ctx.use_parallel: | ||
| if ctx.use_parallel and ctx.content_length >= self._config.files_ext_multipart_upload_min_stream_size: |
There was a problem hiding this comment.
Maybe, not in this PR. Let's avoid using the same uploadContext in both methods, as they may differ for different functions.
FWIW, I am planning to add a static type checker in the Python SDK, which will block such scenarios.
yuanjieding-db
force-pushed
the
yuanjie/fix-zero-size-file-upload
branch
from
fce05cc to
e15bcb5
Compare
yuanjieding-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
yuanjieding-db
force-pushed
the
yuanjie/fix-zero-size-file-upload
branch
from
e15bcb5 to
504f30c
Compare
yuanjieding-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
2 participants
What changes are proposed in this pull request?
Provide the readers and reviewers with the information they need to understand
this PR in a comprehensive manner.
Specifically, try to answer the two following questions:
FilesExtissue where it throws exception when the content size is 0, by checking if the size of the content is below a certain threshold. If it is below the threshold, upload using single-shot instead of multipart upload.uploadinterface needs to support upload data with zero size.How is this tested?
Unit test cases are updated to test the behavior change.