Workaround for query-consistency-strong which is presently partially broken #5928
Conversation
Codecov Report
Additional details and impacted files
... and 10 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Pull Request Test Coverage Report for Build 018f0def-7272-4cb6-b153-4744cb4d8f4cDetails
💛 - Coveralls |
davidporter-id-au
requested review from
Shaddoll,
neil-xie,
Groxx,
shijiesheng,
agautam478,
jakobht,
3vilhamster,
sankari165,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
davidporter-id-au
changed the title
| // info correctly making this call erroneously fail sometimes. | ||
| // https://t3.uberinternal.com/browse/CDNC-8641 | ||
| // So defaulting just this flow to fail-open in the absence of headers. | ||
| if versionErr != nil && clientImpl != "" && clientFeatureVersion != "" { |
There was a problem hiding this comment.
Hmm sure, this is fine for now
There was a problem hiding this comment.
there are code paths within vesionChecker that handles clientImpl == nil gracefully. so expecting both clientImpl and clientFeatureVersion to be empty might exclude some genuine cases.
There was a problem hiding this comment.
discussed offline, I believe we concluded that the clients that didn't support this and were using QueryConsistencyStrong to be a small set.
davidporter-id-au
merged commit 44b1f59
into
cadence-workflow:master
What changed?
As per comment discription, this is a workaround for an apparently outstanding bug where it looks like headers are not being passed along on all query paths. The problem remains, but to prevent a customer requiring this feature for now this removes the check on this one specific part of flow.
Obviously the correct and more ideal fix is to fix the missing headers, however, that'll take a bit of time and time-being what it is I'm trying to unblock Strong queries for this user.
Why?
How did you test it?
Potential risks
Release notes
Documentation Changes