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

bump aws-sdk to v3 for s3 #10363

Merged
merged 2 commits into from
Mar 23, 2023
Merged

Conversation

yszkst
Copy link
Contributor

@yszkst yszkst commented Mar 19, 2023

What

aws-sdk を v3 へ更新  
S3Service, DriveServiceのコード・テスト更新

Why

#10170

Additional info (optional)

GoogleCloudStorage (GCS)

存在しないキーへのDelete

GCS は 404 を返すので NoSuchKey 扱い
S3, MinIO, Wasabi は 204 を返すので正常扱い

MultipartUpload

GCS は2021年よりMultipartUploadに対応しているがチャンク数上限が32個なのとaws-sdkから実行すると レスポンス パラメーターのキーが合わずエラーになるのでpartSizeを引き上げる動作は変えていない。

  Code: 'MalformedCompleteMultipartUploadRequest',
  Details: 'XML not conformant to the schema at line: 1 column: 113.  element "CompleteMultipartUpload" incomplete; missing required element "Part"'

S3Client取得をupload()/delete()の中に入れたが強い理由はない。

S3Serviceは薄くて、DriveServiceでdeleteのテストがあるのでS3ServiceのUnitには作らなかった。

ほとんどのアップロードが10MB以下の画像・音声で、上限を100MBのように決めてしまうならMultipartUploadでなくPutObjectにしてもよい気がした。

GCSのクセがつよい。

追記
32 は 並列複合アップロード https://cloud.google.com/storage/docs/parallel-composite-uploads?hl=ja とXML API マルチパートアップロード https://cloud.google.com/storage/docs/multipart-uploads?hl=ja を混同していたかもしれない。

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added packages/backend Server side specific issue/PR 🧪Test labels Mar 19, 2023
@codecov
Copy link

codecov bot commented Mar 19, 2023

Codecov Report

Merging #10363 (2e17856) into develop (37b8f40) will increase coverage by 0.01%.
The diff coverage is 86.95%.

@@             Coverage Diff             @@
##           develop   #10363      +/-   ##
===========================================
+ Coverage    74.84%   74.86%   +0.01%     
===========================================
  Files          876      876              
  Lines        85517    85541      +24     
  Branches      5743     5753      +10     
===========================================
+ Hits         64009    64040      +31     
+ Misses       21508    21501       -7     
Impacted Files Coverage Δ
packages/backend/src/core/DriveService.ts 60.28% <72.00%> (+0.99%) ⬆️
packages/backend/src/core/S3Service.ts 97.10% <95.45%> (+1.86%) ⬆️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@syuilo syuilo merged commit 658901a into misskey-dev:develop Mar 23, 2023
@syuilo
Copy link
Member

syuilo commented Mar 23, 2023

🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants