Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/297#issuecomment-70413324
Thanks for taking a go at this. It is going into a good direction.
What needs to be adjusted is the way that the job manager gets accessed.
The current code creates a new jobmanager (that is just sitting there
doing nothing). It really needs to connect to an existing jobmanager
by getting an ActorRef to it.
I think it would be nice to split this large method up into multiple
methods that serve different information at different URLs. That would
be more REST style. Something like
```java
public class JobManagerInfoService {
private static final Logger LOG =
LoggerFactory.getLogger(JobManagerInfoService.class);
private final ActorRef jobManager;
private final ActorRef archive;
private final FiniteDuration timeout;
public JobManagerInfoService( ... ) {
...
}
@Path(/taskmanagers)
public class TaskManagerAction {
@GET
@Produces(application/json)
public String getTaskManagers(HttpServletRequest request) {
...
}
}
@Path(/cancelJob)
public class TaskManagerAction {
@GET
@Produces(application/json)
public String getTaskManagers(HttpServletRequest request,
@QueryParam(job) String jobId) {
...
}
}
}
```
Low priority:
- Is it possible to create the JSON strings using a library? The manual
ceation is always error prone. I think that the
Jackson JSON library is being used in other parts of Flink anyways.
- When referencing Scala case object from Java (such as the
JobManagerMessages), it is better to create a utility method in the
declaring scala class to obtain them (similar as in the class
`TaskManagerMessages`). This helps both the code readability,
and some IDEs. Eclipse JDT often has a problems (like a bug) resolving
class names containing a `$` symbol.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
