Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4734
Hi @tillrohrmann, this PR is now rebased to the latest master, and reworked
to incorporate your last comments.
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4734
@tillrohrmann yes, reusing `DispatcherGateway#requestJobDetails` was an
option that did occur to me but was also not sure of the redundant cost. But
sure, we can avoid adding yet another RPC for
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/4734
Maybe you could rebase your work on top of
https://github.com/tillrohrmann/flink/tree/FLINK-7870. That's the branch where
I stacked multiple of the pending handler PRs and which I will merge
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/4734
Thanks a lot @tzulitai. Will take another look at it and merge it if I have
no other comments.
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4734
@tillrohrmann the PR is now rebased. Previous comments have also been
addressed.
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4734
Yes, I'll rebase this now.
---
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/4734
I think @zentol is right and we can register the
`CurrentJobsOverviewHandler` under `jobs/overview`. Thus, we should add this
handler as well. Could you rebase this handler onto the latest
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/4734
@tzulitai, I actually want to re-check whether it's indeed not possible to
register the `CurrentJobsOverviewHandler` under `jobs/overview`. If this should
indeed be the case, then it will most
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4734
Hi @tillrohrmann, just to double check, so does the conclusion mean that
#4805 subsumes this PR, and this one can be closed?
---
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/4734
@tzulitai, we actually figured out that we cannot register the
`CurrentJobsOverviewHandler` under `/jobs/overview` because it conflicts with
`/jobs/:jobid`. Therefore, we thought that we could
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4734
@tillrohrmann @zentol alright, thanks for the review and inputs. I'll
address comments and change the `CurrentJobIdsHandler` to return all available
Job IDs as a simple list as Till suggested.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/4734
I just talked to @zentol and we both agreed that we should register the
`CurrentJobsOverviewHandler` under `jobs/overview`. Moreover, we should change
the `CurrentJobIdsHandler` such that it
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/4734
I agree with @zentol. Moreover, I would like to change
`MultipleJobsDetails` to not split the job details into running and finished.
Just a collection of `JobDetails`.
---
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4734
Actually, I'd like to throw out the entire handler and replace it with the
`CurrentJobsOverviewHandler`. The CurrentJobIdshandler handler is _not_ used by
the web UI as the listings of jobs in the UI
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4734
nvm, this isn't used by the web UI:
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4734
The additional job statuses are only new JSON fields. Not sure how the web
UI handles them, but shouldn't the change be non-breaking?
We could also revert b60466c for now if that will make
16 matches
Mail list logo
