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

[#125] Fixed bucket creation #143

Merged
merged 5 commits into from
Jul 9, 2021
Merged

[#125] Fixed bucket creation #143

merged 5 commits into from
Jul 9, 2021

Conversation

KirillovDenis
Copy link
Contributor

closes #125

Signed-off-by: Denis Kirillov denis@nspcc.ru

Bucket should has unique name.

Signed-off-by: Denis Kirillov <denis@nspcc.ru>
Container must be public (basic acl) to enable bearer token.

Signed-off-by: Denis Kirillov <denis@nspcc.ru>
Signed-off-by: Denis Kirillov <denis@nspcc.ru>
if err != nil {
errMsg := err.Error()
if strings.Contains(errMsg, "bucket not found") ||
strings.Contains(errMsg, "container not found") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do better here?

I think GetBucketInfo function can return one predefined error if bucket was not found and we can use errors.Is on that. I also see, that GetBucketInfo returns grpc related status error, which doesn't make sense to me. Maybe I am missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose grpc related status is used to form more precisely http response status in HeadBucketHandler.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah. But gRPC status codes are not directly related to HTTP status codes.

Probably the usage of gRPC statuses was inspired by NeoFS transport layer (it uses gRPC). However NeoFS does not define single transport library and not limited to gRPC. In fact NeoFS API Client tries to hide transport layer details.

I think we should not use gRPC statuses. This repository should define consistent way how it transform API errors into S3 related HTTP errors, that's it. I propose to discuss it in #149.

Signed-off-by: Denis Kirillov <denis@nspcc.ru>
Signed-off-by: Denis Kirillov <denis@nspcc.ru>
@alexvanin alexvanin merged commit 61ee925 into nspcc-dev:master Jul 9, 2021
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.

Deletion errors on benchmark rerun
2 participants