Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(gcs): add support for gcs append #1801

Merged
merged 7 commits into from
Apr 18, 2023
Merged

Conversation

cuichenli
Copy link
Contributor

Fixes #1452

@cuichenli cuichenli marked this pull request as draft March 30, 2023 07:07
@Xuanwo
Copy link
Member

Xuanwo commented Apr 1, 2023

Hello @cuichenli, thank you for submitting this PR!

I will temporarily close the PR to give you more space to improve it. By closing on-going (instead of on-reviewing) PR allow both of us foucing our own work. Please note that closing the PR does not mean rejection; feel free to reopen it when you are ready for further review or feedback.

Thank you again and have a great day!

@Xuanwo Xuanwo closed this Apr 1, 2023
@Xuanwo Xuanwo reopened this Apr 14, 2023
@Xuanwo
Copy link
Member

Xuanwo commented Apr 14, 2023

I apologize for closing this pull request. Our policy has been updated, and we will no longer close pull requests so hastily. Additionally, I have recently learned that once a maintainer closes a pull request, it cannot be reopened.

@cuichenli
Copy link
Contributor Author

I apologize for closing this pull request. Our policy has been updated, and we will no longer close pull requests so hastily. Additionally, I have recently learned that once a maintainer closes a pull request, it cannot be reopened.

No problem at all! I apologize for the delay in updating the PR as I have been busy with other tasks.

I have made some changes to use the streaming API in the latest commit. However, I must admit that the implementation is not optimal. The main issue arises during the final chunk, where we need to specify the final chunk size. Since this size is unknown until the user invokes the close function, the implementation became a bit tricky. I attempted to send a POST request with an empty body, but unfortunately, it did not work. As an alternative, I recorded the last copy sent and when the user invokes the close function, I resend it with the correct headers. I am uncertain whether this approach is acceptable, but I cannot think of a better solution at this time.

Please take a look. Thanks

@cuichenli
Copy link
Contributor Author

I have updated based on the comments, please review.

@cuichenli cuichenli marked this pull request as ready for review April 18, 2023 13:24
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Great! Thanks a lot!

@Xuanwo Xuanwo merged commit da94856 into apache:main Apr 18, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add append writer support for gcs
2 participants