make populateUncoveredIntervals a configuration in query context#2332
make populateUncoveredIntervals a configuration in query context#2332nishantmonu51 merged 1 commit intoapache:masterfrom
Conversation
There was a problem hiding this comment.
ah I see. this is a limit not a boolean. can this be uncoveredIntervalsLimit or similar?
There was a problem hiding this comment.
so that user can limit the size even if enabled.
There was a problem hiding this comment.
sure, i can rename the key to uncoveredIntervalsLimit if that is more appropriate.
There was a problem hiding this comment.
@drcrallen renamed the property to uncoveredIntervalsLimit
himanshug
force-pushed
the
configurable_partial
branch
2 times, most recently
from
97b8f2a to
ccc35af
Compare
There was a problem hiding this comment.
are we hitting this limit in the test ? If not can you add a test where we actually hit the limit.
There was a problem hiding this comment.
@nishantmonu51 updated test to hit the limit as well.
himanshug
force-pushed
the
configurable_partial
branch
from
ccc35af to
3719b6e
Compare
|
@nishantmonu51 @drcrallen all comments addressed. BTW: i don't think this PR is "Incompatible" , "uncoveredInterval" was not part of 0.8.3 and in 0.9.0 default behavior is same as earlier. |
|
@himanshug I am just wondering whether there is any particular reason for "uncoveredInterval" to be populated in a HTTP header in place of the HTTP body content? |
|
@CHOIJAEHONG1 because it was originally intended to be for information purposes, in future, I guess we might have a separate section in the response payload for things like this. this PR is only intended to fix the bug really. |
|
@himanshug this changes default behavior. If people want the behavior of reporting uncovered intervals, then they need to set a new configuration option |
|
Prior to this patch: return uncovered intervals (or try to) With this patch: Do not return uncovered intervals With this patch AND |
|
my bad on the uncovered intervals thing I thought we already had that in 8.3 |
|
👍 |
1 similar comment
|
👍 |
make populateUncoveredIntervals a configuration in query context
5 participants
fixes #2108