feat: tms readiness check bypass implementation #1920
Conversation
|
|
|
signed https://cla-assistant.io/grafana/loki?pullRequest=1920, status is still |
Codecov Report
@@ Coverage Diff @@
## master #1920 +/- ##
==========================================
+ Coverage 64.64% 64.75% +0.11%
==========================================
Files 125 125
Lines 9534 9534
==========================================
+ Hits 6163 6174 +11
+ Misses 2939 2932 -7
+ Partials 432 428 -4
|
|
@cyriltovena can u please take a look ^^ |
|
I'm good with the approach but I don't like the current config/variable naming. We should use something like health_check_targets which should default to true |
|
Also can you try again the CLA ? |
|
@cyriltovena sounds good, will make the requested changes and will try the cla again, thank you for looking into this PR that quick! |
|
@cyriltovena addressed comments, for the CLA, it says "You have signed the CLA for grafana/loki" when I click Details, did it multiple times |
|
hey @alexanderGalushka thanks for the PR!, usually CLA issues related to the email address used on the commits vs the email(s) you have configured in github. Your GH profile shows a gmail address however the last commit has a different email associated with it. I think you likely need to add the other email to your GH profile, or re-author the last commit with your gmail (or push a new commit with your gmail) and the CLA will be happy! |
pkg/promtail/server/server.go
Outdated
| if s.healthCheckTarget { | ||
| if !s.tms.Ready() { |
There was a problem hiding this comment.
Sorry to snoop around here. You can do if s.healthCheckTarget && !s.tms.Ready(). You don't need two if statements.
There was a problem hiding this comment.
@adityacs addressed
@slim-bean last commit is signed with my gmail, when you follow the link it says signed, although in the review it reports as not signed. |
|
hrm, would you be able to squash your 3 commits into 1 and force push? |
yeah, was thinking to do that :) |
alexanderGalushka
force-pushed
the
ag_readiness_tms_bybass
branch
from
6b4c9d4
to
403fad2
Compare
|
@slim-bean squashed and forced pushed |
|
unfortunately when I look at the log for that commit it is showing the non gmail email |
|
@slim-bean this half baked cla bot doesn't really spark the joy :( |
|
@slim-bean is it good to be merged as is? |
|
can you try ammending your last commit to fix the email to use an email address you have linked to your github account?:
|
fix: renamed flag to health_check_target, default to true fix: flattened nested if statements
alexanderGalushka
force-pushed
the
ag_readiness_tms_bybass
branch
from
403fad2
to
ad21f03
Compare
|
@slim-bean I was doing exactly that right at the moment you've suggested to try amend, it's green now, thank you for being patient and thank you for your support :) |
|
wooHooo! @alexanderGalushka thanks for your patience as well. It's not too often when the CLA bot fights back but when it does it seems to go about as well as this :/ it's annoying it wouldn't accept the added commit after you updated the email. Oh well, glad we got it sorted out! |
What this PR does / why we need it:
Implements tms readiness check bypass for /ready endpoint handler
Which issue(s) this PR fixes:
Fixes #1919
Checklist