rmetzger commented on PR #29136:
URL: https://github.com/apache/flink/pull/29136#issuecomment-5908852183
Can you address these two?
```
Fix before merge
1. The description and commit message are out of date. Your comment led the
author to move the fix into queryJobMastersForInformation, so GET /overview now
fails too. The description still says "requestClusterOverview is unchanged".
These callers are also affected but not mentioned:
- GET /jobs (JobIdsHandler.java:60)
- MiniCluster#listJobs and #requestClusterOverview (MiniCluster.java:878,
:1154)
- MetricFetcherImpl.java:142
The 4 commits should be squashed into one. The first commit adds a
failing test on its own, and the last one is just called "PR comments". The
REST behavior changes from "200 with a partial list" to "500", so a release
note on the Jira ticket would help.
2. No test covers the /overview change. DispatcherTest has no test for
requestClusterOverview at all. TestingJobManagerRunner#requestJobStatus
(TestingJobManagerRunner.java:113) always returns a completed future, so the
author needs a hook like setJobDetailsFutureFunction for it.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
