Skip to content
This repository has been archived by the owner on Dec 14, 2023. It is now read-only.

Don't wrap errors from unexported methods in core.go #6

Merged
merged 1 commit into from
Aug 13, 2020
Merged

Don't wrap errors from unexported methods in core.go #6

merged 1 commit into from
Aug 13, 2020

Conversation

stoyanr
Copy link
Contributor

@stoyanr stoyanr commented Aug 12, 2020

What this PR does / why we need it:
Avoids wrapping errors from unexported methods in core.go. This is undesirable for 2 reasons:

  • Since these errors come from the same package, wrapping them doesn't add much additional context.
  • Wrapping the error from getVM changes the type of the returned error, which causes an Internal instead of NotFound error to be returned if a machine is not found, which breaks the machine creation flow.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

NONE

@stoyanr stoyanr requested a review from a team as a code owner August 12, 2020 16:34
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 12, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 12, 2020
@moadqassem
Copy link
Contributor

/lgtm

@gardener-robot gardener-robot added the reviewed/lgtm Has approval for merging label Aug 13, 2020
moadqassem
moadqassem approved these changes Aug 13, 2020
View reviewed changes
Copy link
Contributor

@moadqassem moadqassem left a comment

Choose a reason for hiding this comment

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

LGTM

@moadqassem moadqassem merged commit 1f3792a into master Aug 13, 2020
@moadqassem moadqassem deleted the dont-wrap-errors branch August 13, 2020 13:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@moadqassem moadqassem moadqassem approved these changes

Assignees
No one assigned
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@stoyanr @moadqassem @gardener-robot-ci-1 @gardener-robot-ci-3 @gardener-robot