-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix possible NPE in RestTasksAction.java #17778
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 possible NPE in RestTasksAction.java #17778
Conversation
Signed-off-by: Ilmar S. Habibulin <[email protected]>
❕ Gradle check result for adc9466: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17778 +/- ##
============================================
- Coverage 72.46% 72.45% -0.01%
- Complexity 66502 66533 +31
============================================
Files 5408 5408
Lines 308080 308155 +75
Branches 44720 44737 +17
============================================
+ Hits 223239 223273 +34
- Misses 66536 66642 +106
+ Partials 18305 18240 -65 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I think it would be pretty easy to improve the unit tests to hit this scenario. I think if you add a TaskInfo to be returned in the response of the unit test here that will work. Something like: final TaskInfo taskInfo = new TaskInfo(
new TaskId("test-node-id", randomLong()),
"test_type",
"test_action",
"test_description",
null,
randomLong(),
randomLong(),
false,
false,
TaskId.EMPTY_TASK_ID,
Map.of("foo", "bar"),
randomResourceStats(randomBoolean())
);
listener.onResponse((Response) new ListTasksResponse(List.of(taskInfo), emptyList(), emptyList())); |
Signed-off-by: Ilmar S. Habibulin <[email protected]>
Sorry, can't deepdive right now in OS internals, just copypasted your proposal. |
❌ Gradle check result for aa22955: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Thanks @ngr-ilmarh! This looks good, but there are some missing imports plus a bug with one of those random longs in the test. For some reason I'm not able to push a commit to this pull request, but you can either fix up those errors or cherry pick this commit from my user fork: andrross@854e0cd |
OK, will be back next Monday, thanks for example |
Signed-off-by: Ilmar S. Habibulin <[email protected]>
❌ Gradle check result for 065de7c: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
I've used my organisation repository for this PR, i think this is the reason, why you are unable to push directly @andrross |
❌ Gradle check result for de80ca0: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Thanks @ngr-ilmarh! This looks good. There are indeed flaky tests unrelated to this change. I'll make sure the tests pass and get this merged. |
❌ Gradle check result for de80ca0: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Description
Fix possible NPE in _cat/tasks reply table preparation in case of node, that left cluster during query process.
No tests were run, i think this is useless here
Related Issues
Resolves #17762
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.