Skip to content
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

GSP-826: Add Multipart HTTP Signer Support #826

Merged
merged 7 commits into from
Sep 27, 2021

Conversation

abyss-w
Copy link
Contributor

@abyss-w abyss-w commented Sep 26, 2021

No description provided.

@abyss-w abyss-w changed the title Add Multipart HTTP Signer Support GSP-826: Add Multipart HTTP Signer Support Sep 26, 2021
@abyss-w abyss-w requested review from Xuanwo and a team September 26, 2021 09:22
@abyss-w
Copy link
Contributor Author

abyss-w commented Sep 26, 2021

I have opened a draft PR, please point out any problems, and I will fix them.

@Xuanwo
Copy link
Contributor

Xuanwo commented Sep 26, 2021

We need to support Delete in storage_http_signer too: https://forum.beyondstorage.io/t/topic/226

@abyss-w
Copy link
Contributor Author

abyss-w commented Sep 26, 2021

We need to support Delete in storage_http_signer too: https://forum.beyondstorage.io/t/topic/226

Got it!

BeyondStorage
1、CreateMultipartWithContext(ctx context.Context, path string, pairs …Pair) (o *Object, err error),Need to support Expires parameter 2、add AbortMultipart interface 3、Writemultipart support Query Sign

@abyss-w abyss-w marked this pull request as ready for review September 27, 2021 02:21
@Xuanwo
Copy link
Contributor

Xuanwo commented Sep 27, 2021

Mostly LGTM, please create a tracking issue before @JinnyYi makes the final decision.

@JinnyYi
Copy link
Contributor

JinnyYi commented Sep 27, 2021

I have one more question. Do we need to add QuerySignHTTPAbortMultipart instead of supporting it in QuerySignHTTPDelete? If we keep QuerySignHTTPDelete, should List(QuerySignHTTPList instead of QuerySignHTTPListMultipart) also be consistent?

@Xuanwo
Copy link
Contributor

Xuanwo commented Sep 27, 2021

Do we need to add QuerySignHTTPAbortMultipart instead of supporting it in QuerySignHTTPDelete

No, I think we don't need to. As Delete with multipart id is the same with AbortMultipart.

If we keep QuerySignHTTPDelete, should List(QuerySignHTTPList instead of QuerySignHTTPListMultipart) also be consistent?

I think they are different. List supports ListModePart to list Multipart objects, but ListMultipart is used to list parts inside a multipart object.

@JinnyYi JinnyYi enabled auto-merge (squash) September 27, 2021 06:43
@JinnyYi JinnyYi disabled auto-merge September 27, 2021 06:44
@JinnyYi JinnyYi merged commit 406f3e8 into beyondstorage:master Sep 27, 2021
@abyss-w abyss-w deleted the multipart-http-signer branch September 27, 2021 07:09
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.

3 participants