-
Notifications
You must be signed in to change notification settings - Fork 175
Add fall back to all exceptions encountered when presigned URL is not available for upload and download in FilesExt #1115
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?
Conversation
parthban-db
left a comment
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.
Can you also add this change in the changelog?
| len(pre_read_buffer), | ||
| session_token, | ||
| BytesIO(pre_read_buffer), | ||
| is_first_part=True, |
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.
Do we need to pass this information? Every error returned by this will fall back to FilesAPI, right?
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.
We do need this information for two reasons:
- Whenever it is not possible to use Presigned URL, the SDK should fail at the first part. Otherwise client should retry.
- For uploading of unseekable streams, if the function fails at any part other than the first part, the previous parts would been dropped from memory and thus not possible to perform a fallback. In that case the SDK has no option but throw an exception, and the client should perform a retry.
|
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. |
What changes are proposed in this pull request?
WHAT
FilesExtWHY
How is this tested?
Unit tests are updated to reflect the change.