-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(banzai/KafkaCluster): Update health.lua #15962
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@OneCricketeer thanks for your help with this! I left some questions, mostly for my understanding. Thanks in advance for your patience! (:
resource_customizations/kafka.banzaicloud.io/KafkaCluster/health.lua
Outdated
Show resolved
Hide resolved
Tried running this locally with lua, counter = 0
brokers = 0
for _, broker in pairs(obj.status.brokersState) do
if broker.configurationState == "ConfigInSync" then
if broker.gracefulActionState.cruiseControlState == "GracefulUpscaleSucceeded" or broker.gracefulActionState.cruiseControlState == "GracefulDownscaleSucceeded" then
counter = counter + 1
end
end
brokers = brokers + 1
end
if counter == brokers then |
@ecojan Thanks, but I'm not so sure? It's a table length operator. You can look at my commit history, and you'll see I tried the explicit counter, and the tests were still failing with that, so something else is causing the tests to fail, and I'm just not seeing it. Ref https://en.m.wikibooks.org/wiki/Lua_Programming/length_operator I converted my logic to Python even, and I cannot reproduce the failure I may need to look more at how I can setup this test bed locally and setup a debugger |
Ah, I see.
Apologies. Also see Stackoverflow answers simply suggesting not to use |
Uses ipairs() to correctly iterates over indexed elements Signed-off-by: Jordan Moore <1930631+OneCricketeer@users.noreply.github.com>
Signed-off-by: Jordan Moore <1930631+OneCricketeer@users.noreply.github.com>
Signed-off-by: Jordan Moore <1930631+OneCricketeer@users.noreply.github.com>
Signed-off-by: Jordan Moore <1930631+OneCricketeer@users.noreply.github.com>
Signed-off-by: Jordan Moore <1930631+OneCricketeer@users.noreply.github.com>
Signed-off-by: Jordan Moore <1930631+OneCricketeer@users.noreply.github.com>
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15962 +/- ##
=======================================
Coverage 49.62% 49.62%
=======================================
Files 267 267
Lines 46479 46479
=======================================
Hits 23063 23063
Misses 21144 21144
Partials 2272 2272 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Can we merge this PR?
fix(banzai/KafkaCluster): Update health.lua (argoproj#15962) Uses ipairs() to correctly iterates over indexed elements Signed-off-by: Jordan Moore <1930631+OneCricketeer@users.noreply.github.com>
fix(banzai/KafkaCluster): Update health.lua (argoproj#15962) Uses ipairs() to correctly iterates over indexed elements Signed-off-by: Jordan Moore <1930631+OneCricketeer@users.noreply.github.com> Signed-off-by: jmilic1 <70441727+jmilic1@users.noreply.github.com>
fix(banzai/KafkaCluster): Update health.lua (argoproj#15962) Uses ipairs() to correctly iterates over indexed elements Signed-off-by: Jordan Moore <1930631+OneCricketeer@users.noreply.github.com>
fix(banzai/KafkaCluster): Update health.lua (argoproj#15962) Uses ipairs() to correctly iterates over indexed elements Signed-off-by: Jordan Moore <1930631+OneCricketeer@users.noreply.github.com>
fix(banzai/KafkaCluster): Update health.lua (argoproj#15962) Uses ipairs() to correctly iterates over indexed elements Signed-off-by: Jordan Moore <1930631+OneCricketeer@users.noreply.github.com>
Updates the Lua logic to more correctly count broker objects
Closes #15963
Checklist: