Skip to content

Conversation

OlegLaban
Copy link
Contributor

No description provided.

@jiuker jiuker requested a review from Copilot June 5, 2025 04:03
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Introduces named constants for common S3 error codes and updates the codebase to use them for consistency.

  • Declares constants for standard AWS S3 error codes.
  • Refactors s3ErrorResponseMap and error-generation functions to use these constants.
  • Updates tests to replace raw string literals with the new constants.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
s3-error.go Added error code constants and switched map keys to use those constants
api-error-response.go Refactored HTTP-to-error mapping and helpers to reference constants and s3ErrorResponseMap
api-error-response_test.go Updated test cases to use error code constants instead of literal strings
Comments suppressed due to low confidence (3)

api-error-response.go:221

  • No test covers errTransferAccelerationBucket; consider adding a unit test to validate this error scenario.
func errTransferAccelerationBucket(bucketName string) error {

api-error-response.go:279

  • No test covers the errAPINotSupported helper; consider adding tests to ensure its behavior and message are correct.
func errAPINotSupported(message string) error {

api-error-response.go:212

  • The region-mismatch branch isn't covered by existing tests; add a test to verify the formatted region mismatch message.
if errResp.Code == InvalidRegion && errResp.Region != "" {

@klauspost
Copy link
Contributor

Would it be much work to add the rest? Seems a bit random which are added and which are not.

@OlegLaban OlegLaban force-pushed the add-constants branch 2 times, most recently from b787d12 to 822cbbd Compare June 5, 2025 11:36
@OlegLaban
Copy link
Contributor Author

Would it be much work to add the rest? Seems a bit random which are added and which are not.

I guess no, I will be add them soon.

@OlegLaban OlegLaban force-pushed the add-constants branch 2 times, most recently from 5756273 to 241e2a1 Compare June 6, 2025 21:50
Added dot for error sentence

Added dots for errors messages

Review all error codes

add minio constants in functional_tests

Fix error message in functional tests

move static err messages to constants
Copy link
Contributor

@klauspost klauspost left a comment

Choose a reason for hiding this comment

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

lgtm

@harshavardhana harshavardhana merged commit dc94dff into minio:master Jun 9, 2025
5 checks passed
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.

3 participants