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
refactor(orc8r): replace deprecated errors module #12724
refactor(orc8r): replace deprecated errors module #12724
Conversation
Thanks for opening a PR! 💯
Howto
More infoPlease take a moment to read through the Magma project's
If this is your first Magma PR, also consider reading
|
fac2483
to
b374fbe
Compare
b374fbe
to
9c4f6d7
Compare
3364a91
to
ca1c648
Compare
2c040ba
to
eebff2b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look good to me.
@@ -15,9 +15,9 @@ package southbound | |||
|
|||
import ( | |||
"context" | |||
"errors" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cases in which std lib "errors" is imported without obvious use in the diff, are due to replacing errors.New from pkg/errors by errors.New from std lib, I assume?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly. In the same file you should find a removal of github.com/pkg/errors
@@ -27,7 +28,7 @@ func MarshalConfigs(configs map[string]proto.Message) (ConfigsByKey, error) { | |||
for k, v := range configs { | |||
anyVal, err := ptypes.MarshalAny(v) | |||
if err != nil { | |||
return nil, errors.WithStack(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have no replacement for errors.WithStack
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, you still get the file name and line but no call stack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
eebff2b
to
deafe42
Compare
Replace functions from the github.com/pgk/errors package with `fmt.Errorf` in /magma/orc8r. Created in pairing with Moritz Huebner. Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com>
Replace functions from the github.com/pgk/errors package with `fmt.Errorf` in /magma/lte. Created in pairing with Moritz Huebner. Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com>
deafe42
to
2e759c2
Compare
* refactor(orc8r): replace deprecated errors module Replace functions from the github.com/pgk/errors package with `fmt.Errorf` in /magma/orc8r. Created in pairing with Moritz Huebner. Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com> * refactor(orc8r): replace deprecated errors module Replace functions from the github.com/pgk/errors package with `fmt.Errorf` in /magma/lte. Created in pairing with Moritz Huebner. Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com>
* refactor(orc8r): replace deprecated errors module Replace functions from the github.com/pgk/errors package with `fmt.Errorf` in /magma/orc8r. Created in pairing with Moritz Huebner. Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com> * refactor(orc8r): replace deprecated errors module Replace functions from the github.com/pgk/errors package with `fmt.Errorf` in /magma/lte. Created in pairing with Moritz Huebner. Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com>
Replace functions from the github.com/pgk/errors package with
fmt.Errorf
. Created in pairing with Moritz Huebner.Signed-off-by: Sebastian Wolf sebastian.wolf@tngtech.com
Summary
This PR implements the proposed solutions of ticket #12632 for the orc8r module.
A few instances of broken rollback error handling next to the changes
in this PR are also fixed.
Test Plan
Perform Unit tests
Additional Information
Worked in pairing with @MoritzThomasHuebner