GH-38 validatie record on deletion #72
Conversation
maksymvavilov
force-pushed
the
GH-38
branch
2 times, most recently
from
90f705a to
26aa0b2
Compare
| return ctrl.Result{RequeueAfter: requeueTime}, err | ||
| } | ||
| // requeue for validation | ||
| if requeueTime == validationRequeueTime { |
There was a problem hiding this comment.
Ok so delete was successful but we need to revalidate. Hmm this took me a while to figure out. Could we add a comment explaining that is what is happening here and why we do this comparison?
There was a problem hiding this comment.
added more detailed comment
| } | ||
| return err | ||
| return noRequeueDuration, err |
There was a problem hiding this comment.
does noRequeueDuration have the effect as returning ctrl.Result{}, updateError?
There was a problem hiding this comment.
yes. The way it is interpreted: if err != nil it will ignore ctrl.Result{} entirely and will requeue immediately. If err == nil then it will look at instructions for requeue.
This makes me think that I might made it more confusing than it should be. There was a suggestion to just roam with the boolean flag saying if there were changes published or not and we decide on how to requeue in the very last step. I'm planning on having that implemented after the code freeze (but if that will make more sense could have it ready by the end of a day)
There was a problem hiding this comment.
no leave as is for now and can follow up with improvements
Before removing the finalizer ensure that the DNS provider removed records