refactor: generic resources #681
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
@VanillaSpoon I haven't gone through the whole code but it certainly looks better! :)
| klog.Errorf("[getAppWrapperCompletionStatus] Error unmarshalling, err=%#v", err) | ||
| unstruct, err := genericresource.UnmarshalToUnstructured(objectName.Raw) | ||
| if err != nil { | ||
| klog.Errorf("[getAppWrapperCompletionStatus] Error: %v", err) |
There was a problem hiding this comment.
UnmarshalToUnstructured() also logs this error. Do we need to log it again here?
| unstruct, err := genericresource.UnmarshalToUnstructured(objectName.Raw) | ||
| if err != nil { | ||
| klog.Errorf("[getAppWrapperCompletionStatus] Error: %v", err) |
There was a problem hiding this comment.
Does it make sense to continue the loop iteration if err != nil?
There was a problem hiding this comment.
Thanks for the feedback @ronensc :)
You are correct, we could exit the loop here, as if there is an error returned the remainder of the logic wont be operational.
However, I would be hesitant making adjustments in this pr, It may be worth looking into error handling overall separately.
| namespaced := true | ||
| // todo:DELETEME dd := common.KubeClient.Discovery() | ||
| dd := gr.clients.Discovery() |
There was a problem hiding this comment.
An easy win could be deleting the comment that wants to be deleted :)
| } | ||
|
|
||
|
|
||
| // checks if object has replicas and containers field |
There was a problem hiding this comment.
| // checks if object has replicas and containers field | |
| // hasFields checks if obj has replicas and containers field |
| return isFound, replicas, objContainers | ||
| } | ||
|
|
||
| // checks if object has pod template spec and add new labels |
There was a problem hiding this comment.
| // checks if object has pod template spec and add new labels | |
| // addLabelsToPodTemplateField checks if unstruct has pod template spec and adds new labels |
| name := "" | ||
| // todo:DELETEME dd := common.KubeClient.Discovery() | ||
| dd := gr.clients.Discovery() |
There was a problem hiding this comment.
An easy win could be deleting the comment that wants to be deleted :)
VanillaSpoon
force-pushed
the
GRRefactor
branch
from
9615f65 to
0e6aa32
Compare
Issue link
closes #686
What changes have been made
This Pr is a refactor of the GenericResources within MCAD with the separation of repeated logic into reusable functions, removal of commented out code, and movement of functions to utils, and helper files.
This refactor is aimed and improving readability, and reducing repetition.
Verification steps
Checks