Skip to content

Conversation

@romulets
Copy link
Member

The changed log lines are over alerting right now, it should be a warn instead of error

@romulets romulets requested a review from a team as a code owner January 19, 2026 09:15
@mergify
Copy link

mergify bot commented Jan 19, 2026

This pull request does not have a backport label. Could you fix it @romulets? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

Copy link
Contributor

@amirbenun amirbenun left a comment

Choose a reason for hiding this comment

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

Let's have more clarity on the errors that are returned from AWS SDK.

@romulets romulets requested a review from amirbenun January 19, 2026 15:30
@romulets romulets force-pushed the aws-logging-errors-to-warn branch from dbe86ab to f4fdd05 Compare January 19, 2026 15:51
@romulets romulets force-pushed the aws-logging-errors-to-warn branch from f4fdd05 to 7f48e4c Compare January 19, 2026 15:51
return
}
errMsg := err.Error()
if strings.Contains(errMsg, "NoSuchBucket") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of strings comparison

	var nsb *types.NoSuchBucket
	if errors.As(err, &nsb) {
		p.log.Warnf("Error getting bucket %s for bucket %s: %v", operation, bucketName, err)
		return
	}

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@amirbenun amirbenun left a comment

Choose a reason for hiding this comment

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

Nice!

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.

5 participants