Cleanup error handling in datastoresyncer #70
Conversation
| } | ||
|
|
||
| for _, endpoint := range endpoints { | ||
| if !util.CompareEndpointSpec(endpoint.Spec, d.localEndpoint.Spec) { | ||
| endpointCrdName, err := util.GetEndpointCRDName(endpoint) | ||
| if err != nil { | ||
| klog.Errorf("Error while converting endpoint to CRD Name %s", endpoint.Spec.CableName) | ||
| break | ||
| klog.Errorf("error converting endpoint %#v to CRD Name: %v", endpoint, err) |
There was a problem hiding this comment.
What’s the reasoning here (and elsewhere in the loop) which leads to logging an error rather than returning the error?
There was a problem hiding this comment.
Originally I changed these sites to return error as well but I'm unsure as to the original intent. These are questions went thru my mind: Why was the error above fatal but not this one and the 2 below it... Why does it break here and not below... Are these errors catastrophic enough to propagate and thus exit the process... When deleting, would it return an error if the object doesn't exist... The latter 2 made me squeamish about changing the behavior.
|
There are some conflicts now, sorry. |
4da5a5b to
4802863
Compare
|
I will be merging this as it's just cleanup and I think it will benefit the upcoming release with better error logs. |
- Propagate or log errors instead of calling Fatalf - Propagate errors for functions that return error - Improved logging messages where appropriate Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
ebd7706 to
9a6c398
Compare
|
🤖 Created branch: z_pr70/tpantelis/datastoreresyncer |
|
@tpantelis I updated this one, I'm fine to merge it so we move it out of backlog at this point. |
Thanks - I forgot about this one. |
|
🤖 Closed branches: [z_pr70/tpantelis/datastoreresyncer] |
Signed-off-by: Tom Pantelis tompantelis@gmail.com
This change is