search

konpyutaika / nifikop

The NiFiKop NiFi Kubernetes operator makes it easy to run Apache NiFi on Kubernetes. Apache NiFI is a free, open-source solution that support powerful and scalable directed graphs of data routing, transformation, and system mediation logic.
https://konpyutaika.github.io/nifikop/
Apache License 2.0
126 stars 44 forks source link

Patch infinite request searching #236

Closed juldrixx closed 1 year ago

juldrixx commented 1 year ago
Q A
Bug fix? yes
New feature? no
API breaks? no
Deprecations? no
Related tickets -
License Apache 2.0

What's in this PR?

Change in the management of the UpdateRequest and DropRequest with a new flag to indicate if NiFi can't find the request associated to the ID.

Why?

In some cases, NiFi lost the request (cleaning of the history, etc.) but the operator will be stucked and keep asking NiFi for the request status. So to stop it from spamming NiFi for it, we now say that if NiFi hasn't found the request, we won't ask for it and just move hoping that the request has be done. If not, the operator should find a difference and request the change again.

Additional context

Spam NiFi for nothing and can cause a latency in the operator reconcile loop.

Checklist

mh013370 commented 1 year ago

Do you think we should implement retry logic (say, 3 retries then NotFound)?

juldrixx commented 1 year ago

Do you think we should implement retry logic (say, 3 retries then NotFound)?

Why not, but this number, do you write it as a constant in the code? or make it settable via the resource spec?

mh013370 commented 1 year ago

Do you think we should implement retry logic (say, 3 retries then NotFound)?

Why not, but this number, do you write it as a constant in the code? or make it settable via the resource spec?

Hmm - it's probably easiest kept track of in the CRD Status part of the CRD like you've done here. The limit (say 3x then give up) seems like it should be kept a constant in the code unless we really need to expose it in the resource Spec.

If this feels like too much work and out of scope for this particular bug, we can create an issue and leave it as an enhancement for later.

juldrixx commented 1 year ago

Do you think we should implement retry logic (say, 3 retries then NotFound)?

Why not, but this number, do you write it as a constant in the code? or make it settable via the resource spec?

Hmm - it's probably easiest kept track of in the CRD Status part of the CRD like you've done here. The limit (say 3x then give up) seems like it should be kept a constant in the code unless we really need to expose it in the resource Spec.

If this feels like too much work and out of scope for this particular bug, we can create an issue and leave it as an enhancement for later.

Made the modification with number of retry fixed in the code.

mh013370 commented 1 year ago

@juldrixx : Is it too painful to name the retry field NotFoundRetryCount instead of NotFoundRetry? This way, it's clear that the value is a count.

Otherwise, the impl LGTM!

juldrixx commented 1 year ago

@juldrixx : Is it too painful to name the retry field NotFoundRetryCount instead of NotFoundRetry? This way, it's clear that the value is a count.

Otherwise, the impl LGTM!

Should be good now.