ft: ZENKO-147 Make CRR retry configurable #285
Conversation
bennettbuchanan
force-pushed
the
ft/ZENKO-147/configurableFailedCRRTracking
branch
from
9e380dc
to
42c8748
Compare
|
PR has been updated. Reviewers, please be cautious. |
| @@ -50,6 +50,7 @@ const joiSchema = { | |||
| }, | |||
| topic: joi.string().required(), | |||
| replicationStatusTopic: joi.string().required(), | |||
| trackReplicationFailures: joi.boolean().default(true), | |||
There was a problem hiding this comment.
What do you all think of trackReplicationFailures? Alternatives I considered:
trackCRRFailuresenableReplicationRetry
There was a problem hiding this comment.
I vote for trackCRRFailures
There was a problem hiding this comment.
monitorReplicationFailures?
There was a problem hiding this comment.
As an external parameter name I would prefer sticking to the service semantic, i.e. "replication" service, so trackReplicationFailures. Internally (function names etc.) we also have some names with CRR in it.
There was a problem hiding this comment.
monitorReplicationFailures is good for me too, not being an english native speaker I leave the choice to others 🤣
There was a problem hiding this comment.
Okay, it seems like we all agree on monitorReplicationFailures. 💃We have a winner.
There was a problem hiding this comment.
@jonathan-gramain BTW, your English is awesome! I'm always impressed with your doc writing. 😄
| @@ -192,6 +192,9 @@ class ReplicationStatusProcessor { | |||
| return this.taskScheduler.push({ task, entry: sourceEntry }, | |||
| sourceEntry.getCanonicalKey(), done); | |||
| }); | |||
| } else if (task) { | |||
There was a problem hiding this comment.
The else is not necessary (no big deal but I have memories of the linter complaining about this).
There was a problem hiding this comment.
I vote for removing unnecessary else statements as well.
There was a problem hiding this comment.
Sounds good to me. 😄Done!
bennettbuchanan
force-pushed
the
ft/ZENKO-147/configurableFailedCRRTracking
branch
from
42c8748
to
88e53b9
Compare
ironman-machine
dismissed stale reviews from jonathan-gramain and JianqinWang
Do it again human slave!:point_right: :runner: (Oh and the pull request has been updated, by the way.)
|
PR has been updated. Reviewers, please be cautious. |
There was a problem hiding this comment.
@bennettbuchanan Please make this configurable in Zenko chart as well.
bennettbuchanan
force-pushed
the
ft/ZENKO-147/configurableFailedCRRTracking
branch
from
88e53b9
to
45c9d41
Compare
ironman-machine
dismissed stale reviews from JianqinWang, jonathan-gramain, and rahulreddy
Do it again human slave!:point_right: :runner: (Oh and the pull request has been updated, by the way.)
|
PR has been updated. Reviewers, please be cautious. |
Allow a configurable value to disable tracking CRR failures. 🚀