-
Notifications
You must be signed in to change notification settings - Fork 18
bug : replace event.set_results(success=False) with event.fail() in get-cluster-status #663
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
bug : replace event.set_results(success=False) with event.fail() in get-cluster-status #663
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #663 +/- ##
==========================================
- Coverage 66.25% 63.37% -2.88%
==========================================
Files 17 20 +3
Lines 3180 4623 +1443
Branches 424 759 +335
==========================================
+ Hits 2107 2930 +823
- Misses 935 1445 +510
- Partials 138 248 +110 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| status = self._mysql.get_cluster_status() | ||
|
|
||
| if not status: | ||
| event.fail("Failed to read cluster status. See logs for more information.") |
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.
@arjun11-malik make sure that the output change is not being misinterpreted on the the failing tests
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.
I’ve double-checked every integration test that invokes get-cluster-status—they only wait for the action to complete and then read out results["status"]. None of them inspect the old success=False payload or rely on its exact shape, so switching to event.fail() on empty status won’t change their behavior.
95ea517 to
327a441
Compare
7fe7885 to
4f9ad9d
Compare
|
Arjun is no longer in the company, and the bug-fix has been picked up by @astrojuanlu. |
Issue
Fixes Issue #649
Solution
Replaced the improper usage of
event.set_results(success=False)withevent.fail("...").Also added a unit test for
get-cluster-statusaction to cover both success and failure scenarios.Checklist
Checklist