Skip to content

Conversation

@sanposhiho
Copy link
Member

What this PR does / why we need it:

Currently, minio client gives back no error even if the given object not found when GetObject called.
We should treat as filestore.ErrNotFound like GCS. If we don't, the caller cannot handle it properly:

Which issue(s) this PR fixes:

Fixes #1309

Does this PR introduce a user-facing change?:

NONE

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 33.81%. This pull request does not change code coverage.

@sanposhiho sanposhiho marked this pull request as ready for review January 10, 2021 12:44
if errRes.StatusCode == http.StatusNotFound {
err = filestore.ErrNotFound
}
s.logger.Error("failed to create minio object reader", zap.String("path", path), zap.Error(err))
Copy link
Member

Choose a reason for hiding this comment

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

nit: It would be great if it could be followed our style where must be indented if multiple args given:

	s.logger.Error("failed to create minio object reader",
		zap.String("path", path),
		zap.Error(err),
	)

@nghialv
Copy link
Member

nghialv commented Jan 12, 2021

Thank you.
/approve

@pipecd-bot
Copy link
Collaborator

GO_LINTER

The following files are not gofmt-ed. By commenting /golinter fmt, the formatted one will be appended to this pull request automatically.

pkg/filestore/minio/minio.go
--- pkg/filestore/minio/minio.go.orig
+++ pkg/filestore/minio/minio.go
@@ -106,7 +106,7 @@
 		if e.StatusCode == http.StatusNotFound {
 			err = filestore.ErrNotFound
 		}
-		s.logger.Error("failed to stat minio object", 
+		s.logger.Error("failed to stat minio object",
 			zap.String("path", path),
 			zap.Error(err),
 		)

@sanposhiho
Copy link
Member Author

/golinter fmt

@nghialv
Copy link
Member

nghialv commented Jan 12, 2021

/approve

Co-authored-by: Le Van Nghia <[email protected]>
@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 33.81%. This pull request does not change code coverage.

@nghialv
Copy link
Member

nghialv commented Jan 12, 2021

/approve

@pipecd-bot
Copy link
Collaborator

APPROVE

This pull request is APPROVED by nghialv.

Approvers can cancel the approval by writing /approve cancel in a comment. Any additional commits also will change this pull request to be not-approved.

@pipecd-bot pipecd-bot merged commit 965abdc into master Jan 12, 2021
@pipecd-bot pipecd-bot deleted the handle-errnotfound-on-minio branch January 12, 2021 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Treat no object case as ErrNotFound

5 participants