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

aws-sdk-go-v2V1: EndpointResolverV2: EndpointResolverV2 + BaseEndpoint の形に書き換え #2487

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pikachu0310
Copy link
Contributor

fix(deps): update aws-sdk-go-v2 monorepo #2445
にて、EndpointResolverがDeprecatedとなっておりマージできなくて、調べたらV2に移行しろとの事だったため、以下のドキュメントを見ながらV1: EndpointResolver からV2: EndpointResolverV2 + BaseEndpointに書き換えました。
https://aws.github.io/aws-sdk-go-v2/docs/configuring-sdk/endpoints/

色々調べましたが、分からない所が多いまま修正したため、かなり不安です。

@pikachu0310 pikachu0310 self-assigned this Jul 23, 2024
@pikachu0310
Copy link
Contributor Author

Testが落ちていますが、どう直せばよいのか分からないので、レビューというよりは助けて欲しいです。

config.WithCredentialsProvider(credentials.NewStaticCredentialsProvider("ROOT", "PASSWORD", "")),
)
_ = s3.NewFromConfig(cfg, func(o *s3.Options) {
Copy link
Member

Choose a reason for hiding this comment

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

テスト落ちてる原因はここじゃないかなたぶん

Copy link
Member

Choose a reason for hiding this comment

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

この関数はテスト用のConfig作成用で、aws.Configを構築するためにいろいろしてるんだけど、s3.NewFromConfig はs3 client構築用の関数で、aws.Configに反映するものではないので、EndpointResolverV2の設定が反映されてなさそう

@@ -28,28 +30,39 @@ type S3FileStorage struct {
mutexes *utils.KeyMutex
}

type customResolver struct {
Copy link
Member

Choose a reason for hiding this comment

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

BaseEndpointだけで動く気がしていて、CustomResolverはいらないと思う
もともとカスタムリゾルバっぽくしてたのは、BaseURL書き換えるにはそうするしかなかったから

一応非推奨だし

Because of this, we recommend that you don’t replace the EndpointResolverV2 implementation in your S3 client.

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.

2 participants