fix 4509, remove finalizer#4511
Conversation
knative-prow-robot
added
the
size/M
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vaikas The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
Codecov Report
@@ Coverage Diff @@
## master #4511 +/- ##
==========================================
- Coverage 81.26% 81.26% -0.01%
==========================================
Files 284 284
Lines 8013 8021 +8
==========================================
+ Hits 6512 6518 +6
- Misses 1113 1114 +1
- Partials 388 389 +1
Continue to review full report at Codecov.
|
| if imc.Status.Address != nil && | ||
| imc.Status.Address.URL != nil { | ||
| if hostName := imc.Status.Address.URL.Host; hostName != "" { | ||
| logging.FromContext(ctx).Info("Removing dispatcher") | ||
| r.multiChannelMessageHandler.DeleteChannelHandler(hostName) | ||
| } | ||
| } |
There was a problem hiding this comment.
I meant do it in controller.go, this won't be called on deletions
There was a problem hiding this comment.
I'd do it in a DeleteFunc on:
There was a problem hiding this comment.
I think you also want to fetch the object using this helper (then cast it to IMC): https://github.com/knative/pkg/blob/e9570bdb548d85a0ce1d73f64f348771377ae56f/kmeta/accessor.go#L47
There was a problem hiding this comment.
@vaikas LMK if you wanna sync on this so we can land it 😉
There was a problem hiding this comment.
I'll try to get this done today, working on moving the rabbitmq into the new test infra.
| if imc.Status.Address != nil && | ||
| imc.Status.Address.URL != nil { |
There was a problem hiding this comment.
Is this new line on purpose?
|
/assign |
knative-prow-robot
added
size/L
| } | ||
|
|
||
| func (r *Reconciler) deleteFunc(obj interface{}) { | ||
| if acc, err := kmeta.DeletionHandlingAccessor(obj); err == nil { |
There was a problem hiding this comment.
Generally Go style prefers inverting these and returning early to avoid the deep indentation.
| // If the IMC has been deleted just make sure the handler is removed. | ||
| if !imc.GetDeletionTimestamp().IsZero() { |
There was a problem hiding this comment.
I don't think this will be true on this path because the object we get is the last copy the informer saw, and without a finalizer this won't be set.
|
The following is the coverage report on the affected files.
|
6 participants
Fixes #4509
Proposed Changes
Release Note
Docs