feat: Migrate to aws-sdk-go-v2 #233

Joey-1445601153 · 2021-09-28T09:33:06Z

Fix #86
It's incomplete now.

storage.go

service.go

storage.go

utils.go

Xuanwo · 2021-09-29T08:33:12Z

Please fix all CI here.

Xuanwo · 2021-09-29T08:33:39Z

6           The error should be nil ✘    storager.go:238: delete: Storager s3 {Name: *******, WorkDir: /b5279654-b8cd-4e38-a1ca-d22e45a92382/}, [d7ef011a-d173-4652-b291-1d3c9481b22c]: unexpected: operation error S3: DeleteObject, exceeded maximum number of attempts, 3, https response error StatusCode: 0, RequestID: , HostID: , request send failed, Delete "https://*******.s3.us-west-2.amazonaws.com/b5279654-b8cd-4e38-a1ca-d22e45a92382/d7ef011a-d173-4652-b291-1d3c9481b22c?x-id=DeleteObject": 301 response missing Location header

Looks like we are missing the location.

Xuanwo · 2021-09-30T01:57:08Z

54         The error should be nil ✘✔    storager.go:291: write: Storager s3 {Name: *******, WorkDir: /f573cc53-e0f8-4a60-85ec-7ea6e61f10b8/}, [f19db38f-6934-4b2d-a459-f5adec2718ab]: unexpected: operation error S3: PutObject, exceeded maximum number of attempts, 3, https response error StatusCode: 0, RequestID: , HostID: , request send failed, Put "https://*******.s3.*******.amazonaws.com/f573cc53-e0f8-4a60-85ec-7ea6e61f10b8/f19db38f-6934-4b2d-a459-f5adec2718ab?x-id=PutObject": net/http: HTTP/1.x transport connection broken: http: ContentLength=1333208 with Body length 0

I believe this is why we need cfg.S3DisableContentMD5Validation = aws.Bool(true) before. Does aws-go-sdk-v2 provide similar feature?

Joey-1445601153 · 2021-09-30T02:04:10Z

54         The error should be nil ✘✔    storager.go:291: write: Storager s3 {Name: *******, WorkDir: /f573cc53-e0f8-4a60-85ec-7ea6e61f10b8/}, [f19db38f-6934-4b2d-a459-f5adec2718ab]: unexpected: operation error S3: PutObject, exceeded maximum number of attempts, 3, https response error StatusCode: 0, RequestID: , HostID: , request send failed, Put "https://*******.s3.*******.amazonaws.com/f573cc53-e0f8-4a60-85ec-7ea6e61f10b8/f19db38f-6934-4b2d-a459-f5adec2718ab?x-id=PutObject": net/http: HTTP/1.x transport connection broken: http: ContentLength=1333208 with Body length 0

I believe this is why we need cfg.S3DisableContentMD5Validation = aws.Bool(true) before. Does aws-go-sdk-v2 provide similar feature?

No,I have not found it yet.

Xuanwo · 2021-09-30T02:05:54Z

Maybe we need to use https://github.com/aws/aws-sdk-go-v2/blob/cc132614991d2e36a7b5ca8685aea13bf1337224/aws/signer/v4/middleware.go#L107-L110

GitHub
aws-sdk-go-v2/middleware.go at cc132614991d2e36a7b5ca8685aea13bf1337224 · aws/aws-sdk-go-v2
AWS SDK for the Go programming language. . Contribute to aws/aws-sdk-go-v2 development by creating an account on GitHub.

Xuanwo · 2021-09-30T02:08:30Z

We can call the previous function via

https://github.com/aws/aws-sdk-go-v2/blob/cc132614991d2e36a7b5ca8685aea13bf1337224/service/s3/api_client.go#L131-L135

GitHub
aws-sdk-go-v2/api_client.go at cc132614991d2e36a7b5ca8685aea13bf1337224 · aws/aws-sdk-go-v2
AWS SDK for the Go programming language. . Contribute to aws/aws-sdk-go-v2 development by creating an account on GitHub.

utils.go

Xuanwo · 2021-09-30T03:41:52Z

1   go: github.com/beyondstorage/go-service-s3/v2: package github.com/aws/smithy-go/middleware imported from implicitly required module; to add missing requirements, run:

11:37:51     go get github.com/aws/smithy-go/[email protected]

11:37:51   make: *** [Makefile:35: integration_test] Error 1

11:37:52   Process exited with code 2

Please always run make build before submit.

Joey-1445601153 · 2021-09-30T04:27:06Z

The error should be nil ✘    storager.go:201: delete: Storager s3 {Name: *******, WorkDir: /9b827af8-195e-49a3-a2b9-5e5645e4c296/}, [63386767-902c-4486-8660-7762a504e718]: unexpected: operation error S3: DeleteObject, https response error StatusCode: 400, RequestID: 4S6DEFYWG5MJCF6G, HostID: 2rJAfdfSIdDhwU/EayYcYq7U+l1Gcc/zYXLjugEJpJ74ePullv//mVc+bzNbms1F4ZfnGkHlsUY=, api error InvalidRequest: Missing required header for this request: x-amz-content-sha256

It seems like we have to find a way to replace cfg.LowerCaseHeaderMaps = aws.Bool(true).

Xuanwo · 2021-09-30T04:28:30Z

Maybe you want to use the following style to add code blocks.

```
xxxxx
```

Xuanwo · 2021-10-09T05:55:08Z

service.toml

@@ -4,7 +4,7 @@ name = "s3"

 [namespace.service.new]
 required = ["credential"]
-optional = [ "endpoint", "http_client_options", "force_path_style", "disable_100_continue", "use_accelerate", "use_arn_region"]
+optional = [ "endpoint", "http_client_options", "force_path_style", "location", "disable_100_continue", "use_accelerate", "use_arn_region"]


Why do we need location in this service?

Xuanwo · 2021-10-09T09:06:11Z

ping @JinnyYi for a review.

utils.go

JinnyYi · 2021-10-11T07:49:34Z

storage.go

-
-	getReq, _ := s.service.GetObjectRequest(input)
-	url, headers, err := getReq.PresignRequest(expire)
+	presignClient := s3.NewPresignClient(s.service)


Maybe we need to set PresignOptions like expire?

Maybe we need to set PresignOptions like expire?

Yes, i missed it too!!!
i will check it and fix it right now.

Seems that the configs for presign client could be different with basic client? Maybe we can consider or discuss it later?

@JinnyYi
I found that in aws-sdk-go-v2's test case,they use it like

presignClient := s3.NewPresignClient(s.service, func(options *s3.PresignOptions) { options.Expires = expire })

And now I config it in this way too.

Xuanwo · 2021-10-12T04:05:51Z

Looks much better than the first version!

Joey-1445601153 · 2021-10-12T04:22:54Z

Looks much better than the first version!

Well,thanks to all your reviewers.

Xuanwo · 2021-10-13T03:50:41Z

ping @JinnyYi for the final decision.

JinnyYi · 2021-10-13T05:42:14Z

ping @JinnyYi for the final decision.

I've no more questions.

Maybe we should identify the following issues soon to provide capabilities equivalent to the current version:

zhangjiayu2132 added 2 commits September 28, 2021 17:29

Migrate to aws-sdk-go-v2

0041f14

Migrate to aws-sdk-go-v2

c2972cc

Xuanwo reviewed Sep 28, 2021

View reviewed changes

storage.go Outdated Show resolved Hide resolved

zhangjiayu2132 added 4 commits September 28, 2021 18:33

Migrate to aws-sdk-go-v2

864ca29

Migrate to aws-sdk-go-v2

bf7e4a4

Migrate to aws-sdk-go-v2

23c65b5

Migrate to aws-sdk-go-v2

5cfc3f0

Xuanwo changed the title ~~fix:Migrate to aws-sdk-go-v2~~ feat: Migrate to aws-sdk-go-v2 Sep 29, 2021

Xuanwo reviewed Sep 29, 2021

View reviewed changes

service.go Outdated Show resolved Hide resolved

storage.go Outdated Show resolved Hide resolved

utils.go Outdated Show resolved Hide resolved

zhangjiayu2132 added 5 commits September 29, 2021 10:24

fix wrong usage of convert

1819824

migrate to aws-sdk-go-v2

3011bfa

fix wrong usage of convert

d565e45

process confilct

8a21d6b

process confilct

2e7afe8

Joey-1445601153 marked this pull request as ready for review September 29, 2021 07:57

add Location

aa8d40c

JinnyYi reviewed Sep 30, 2021

View reviewed changes

utils.go Outdated Show resolved Hide resolved

zhangjiayu2132 added 2 commits September 30, 2021 11:29

add RemoveContentSHA256HeaderMiddleware

2034716

add RemoveContentSHA256HeaderMiddleware

a89f428

zhangjiayu2132 added 2 commits September 30, 2021 11:44

run make build

8f5b610

add RemoveContentSHA256HeaderMiddleware

f7ea5a5

zhangjiayu2132 added 4 commits October 9, 2021 13:40

delete extra file .gitattributes

4902d44

Delete location

37d8788

delete location

0905d9d

Delete location

5838592

Xuanwo reviewed Oct 9, 2021

View reviewed changes

zhangjiayu2132 added 4 commits October 9, 2021 14:36

delete location in service.toml

bf18a1d

fix wrong delete of location

fdbb463

add location

7d18ade

fix unsuitable comment

3316c61

Xuanwo requested review from a team October 9, 2021 09:06

xxchan reviewed Oct 9, 2021

View reviewed changes

utils.go Outdated Show resolved Hide resolved

utils.go Outdated Show resolved Hide resolved

zhangjiayu2132 added 3 commits October 10, 2021 18:07

add comment that is deleted by error

b26f5ec

add setting of endpoint

a2594df

Merge branch master into dev

a85a4d0

JinnyYi reviewed Oct 11, 2021

View reviewed changes

utils.go Outdated Show resolved Hide resolved

use endpoint.Parse to avoid getting invalid endpoint

f09eac3

JinnyYi reviewed Oct 11, 2021

View reviewed changes

set expires when creating presignClient

8a27843

zhangjiayu2132 added 2 commits October 13, 2021 16:45

process conflict

672b277

process conflict

eaa3603

JinnyYi merged commit 712be85 into beyondstorage:master Oct 14, 2021

Joey-1445601153 deleted the dev branch October 14, 2021 02:02

BeyondRobot mentioned this pull request Oct 18, 2021

Weekly report since 2021-10-11 beyondstorage/community#57

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Migrate to aws-sdk-go-v2 #233

feat: Migrate to aws-sdk-go-v2 #233

Joey-1445601153 commented Sep 28, 2021 •

edited by Xuanwo

Loading

Xuanwo commented Sep 29, 2021

Xuanwo commented Sep 29, 2021

Xuanwo commented Sep 30, 2021

Joey-1445601153 commented Sep 30, 2021

Xuanwo commented Sep 30, 2021 •

edited by unfurl-links bot

Loading

Xuanwo commented Sep 30, 2021 •

edited by unfurl-links bot

Loading

Xuanwo commented Sep 30, 2021

Joey-1445601153 commented Sep 30, 2021 •

edited

Loading

Xuanwo commented Sep 30, 2021

Xuanwo Oct 9, 2021

Xuanwo commented Oct 9, 2021

JinnyYi Oct 11, 2021

Joey-1445601153 Oct 11, 2021

Joey-1445601153 Oct 11, 2021

JinnyYi Oct 11, 2021

Joey-1445601153 Oct 11, 2021

Xuanwo commented Oct 12, 2021

Joey-1445601153 commented Oct 12, 2021

Xuanwo commented Oct 13, 2021

JinnyYi commented Oct 13, 2021

feat: Migrate to aws-sdk-go-v2 #233

feat: Migrate to aws-sdk-go-v2 #233

Conversation

Joey-1445601153 commented Sep 28, 2021 • edited by Xuanwo Loading

Xuanwo commented Sep 29, 2021

Xuanwo commented Sep 29, 2021

Xuanwo commented Sep 30, 2021

Joey-1445601153 commented Sep 30, 2021

Xuanwo commented Sep 30, 2021 • edited by unfurl-links bot Loading

Xuanwo commented Sep 30, 2021 • edited by unfurl-links bot Loading

Xuanwo commented Sep 30, 2021

Joey-1445601153 commented Sep 30, 2021 • edited Loading

Xuanwo commented Sep 30, 2021

Xuanwo Oct 9, 2021

Choose a reason for hiding this comment

Xuanwo commented Oct 9, 2021

JinnyYi Oct 11, 2021

Choose a reason for hiding this comment

Joey-1445601153 Oct 11, 2021

Choose a reason for hiding this comment

Joey-1445601153 Oct 11, 2021

Choose a reason for hiding this comment

JinnyYi Oct 11, 2021

Choose a reason for hiding this comment

Joey-1445601153 Oct 11, 2021

Choose a reason for hiding this comment

Xuanwo commented Oct 12, 2021

Joey-1445601153 commented Oct 12, 2021

Xuanwo commented Oct 13, 2021

JinnyYi commented Oct 13, 2021

Joey-1445601153 commented Sep 28, 2021 •

edited by Xuanwo

Loading

Xuanwo commented Sep 30, 2021 •

edited by unfurl-links bot

Loading

Xuanwo commented Sep 30, 2021 •

edited by unfurl-links bot

Loading

Joey-1445601153 commented Sep 30, 2021 •

edited

Loading