fix(registry): return error when registry name not found#955
fix(registry): return error when registry name not found#955sahu-virendra-1908 wants to merge 1 commit into
Conversation
|
@bupd @Vad1mo @OrlinVasilev Please review this PR when you have time. I’ve addressed the issue by returning a proper error when the registry name is not found and by propagating the error correctly in the delete flow. If any additional changes or improvements are needed, please let me know. Thanks |
qcserestipy
left a comment
qcserestipy
left a comment
There was a problem hiding this comment.
Thank you for your contribution! In principle LGTM, please fix the small suggestions
| registryID, _ := api.GetRegistryIdByName(arg) | ||
| registryID, err := api.GetRegistryIdByName(arg) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
please return a formated err that also parses the upstream harbor error message. There is utils.ParseHarborErrorMsg that you can use
|
|
||
| return 0, err | ||
| return 0, fmt.Errorf( | ||
| "registry with name %q not found", |
There was a problem hiding this comment.
please fix the format of these new lines
also use utils.ParseHarborErrorMsg
qcserestipy
added
the
Changes Requesed
|
Please also fix your failing DCO check |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #955 +/- ##
=========================================
- Coverage 10.99% 9.27% -1.72%
=========================================
Files 173 290 +117
Lines 8671 14601 +5930
=========================================
+ Hits 953 1354 +401
- Misses 7612 13121 +5509
- Partials 106 126 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Description
Fixes an issue where
GetRegistryIdByNamereturned(0, nil)when a registry name did not exist.Previously, callers interpreted the result as success and continued operating on registry ID
0, causing commands like:to incorrectly call:
This change now returns a proper error when the registry is not found and propagates the error correctly in the delete command.
Type of Change
Changes
GetRegistryIdByNameregistry/delete.go0