chore(feature-tracker): simplifies struct handling #1092
Conversation
Refactors FeatureTracker related functions to plain functions instead of Feature receivers This improves separation of concerns and eliminates unnecessary coupling. Revised the FeatureTracker attachment process to the owning Features.
| if tracker, errGet := getFeatureTracker(ctx, f); client.IgnoreNotFound(errGet) != nil { | ||
| return errGet | ||
| } else { | ||
| associatedTracker = tracker |
There was a problem hiding this comment.
is there any possibility that after associatedTracker = tracker associatedTracker is nil?
otherwise why not just
return client.IgnoreNotFound(f.Client.Delete(ctx, tracker))
There was a problem hiding this comment.
In theory, it can be not found in L62, and so it is nil. We do not propagate error on 404 (client.IgnoreNotFound(errGet)), but we handle this case by simply skipping the Delete operation. Calling Delete on nil will panic.
As to part if this can happen at runtime - under the hood it boils down to which operations are performed directly against kube-apiserver and which are taken from the local cache. I think it can be stale and might lead to such situation, so I thought it's better to handle it explicitly.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zdtsw The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2d2a56a
into
opendatahub-io:incubation
* chore(feature-tracker): simplifies struct handling Refactors FeatureTracker related functions to plain functions instead of Feature receivers This improves separation of concerns and eliminates unnecessary coupling. Revised the FeatureTracker attachment process to the owning Features. * fix: merge conflict (cherry picked from commit 2d2a56a)
* chore(feature-tracker): simplifies struct handling Refactors FeatureTracker related functions to plain functions instead of Feature receivers This improves separation of concerns and eliminates unnecessary coupling. Revised the FeatureTracker attachment process to the owning Features. * fix: merge conflict (cherry picked from commit 2d2a56a)
Description
Refactors
FeatureTracker-related functions to plain functions instead of Feature receivers. This eliminates unnecessary coupling with theFeaturestruct.How Has This Been Tested?
make+ manual with simple KServe setup (which I hope can be part of an automated test suite one day...)Screenshot or short clip
Merge criteria