You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by sachingoel0101 <gi...@git.apache.org> on 2015/10/07 13:54:54 UTC

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

GitHub user sachingoel0101 opened a pull request:

    https://github.com/apache/flink/pull/1236

    [FLINK-2730][web-dashboard] Adds cpu and memory usage graphs.

    Screenshots:
    ![screenshot 21](https://cloud.githubusercontent.com/assets/8874261/10336674/1d1d9e60-6d18-11e5-94f8-7308098789bf.png)
    ![screenshot 22](https://cloud.githubusercontent.com/assets/8874261/10336675/1d2a8bf2-6d18-11e5-9dde-194097ebf103.png)
    ![screenshot 23](https://cloud.githubusercontent.com/assets/8874261/10336676/1d2c22b4-6d18-11e5-8ca5-0abd44349f0b.png)
    ![screenshot 24](https://cloud.githubusercontent.com/assets/8874261/10336677/1d3434f4-6d18-11e5-9b0a-3f95c40d90ae.png)
    
    I would really like to get this in the release. One more step towards removing the old frontend. :) 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/sachingoel0101/flink charts

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/1236.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1236
    
----
commit ba2f6c525fd4c72e1d0468faaef4a9c1476a4eb1
Author: Sachin Goel <sa...@gmail.com>
Date:   2015-10-04T05:59:06Z

    [FLINK-2730][web-dashboard] Adds cpu and memory usage graphs.

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146438040
  
    Thanks for the review @iampeter :smile: 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r44281079
  
    --- Diff: flink-runtime-web/web-dashboard/bower.json ---
    @@ -23,7 +23,8 @@
         "dagre-d3": "~0.4.10",
         "font-awesome": "~4.3.0",
         "moment-duration-format": "~1.3.0",
    -    "qtip2": "~2.2.1"
    +    "qtip2": "~2.2.1",
    +    "highcharts-release": "~4.1.8"
    --- End diff --
    
    Great. I'll start working on d3 based charts after you push your commit.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by iampeter <gi...@git.apache.org>.
Github user iampeter commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146320512
  
    @aljoscha ok will do


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146439447
  
    @aljoscha FYI I've added a new Job Manager message which returns the metrics only for one particular instance to minimize unnecessary traffic for a single taskmanager web page. That okay?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r44274673
  
    --- Diff: flink-runtime-web/web-dashboard/bower.json ---
    @@ -23,7 +23,8 @@
         "dagre-d3": "~0.4.10",
         "font-awesome": "~4.3.0",
         "moment-duration-format": "~1.3.0",
    -    "qtip2": "~2.2.1"
    +    "qtip2": "~2.2.1",
    +    "highcharts-release": "~4.1.8"
    --- End diff --
    
    This library's license is incompatible with the Apache License. Unfortunately, nobody noticed this. We will have to revert the merged changes...


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r44276336
  
    --- Diff: flink-runtime-web/web-dashboard/bower.json ---
    @@ -23,7 +23,8 @@
         "dagre-d3": "~0.4.10",
         "font-awesome": "~4.3.0",
         "moment-duration-format": "~1.3.0",
    -    "qtip2": "~2.2.1"
    +    "qtip2": "~2.2.1",
    +    "highcharts-release": "~4.1.8"
    --- End diff --
    
    I apologize. It should've been my responsibility primarily.
    I think I might be able to achieve the same graphs and live updates using `d3`, which I assume is compatible. Working on it right now.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r44278942
  
    --- Diff: flink-runtime-web/web-dashboard/bower.json ---
    @@ -23,7 +23,8 @@
         "dagre-d3": "~0.4.10",
         "font-awesome": "~4.3.0",
         "moment-duration-format": "~1.3.0",
    -    "qtip2": "~2.2.1"
    +    "qtip2": "~2.2.1",
    +    "highcharts-release": "~4.1.8"
    --- End diff --
    
    Okay. I can a open a PR to minimally remove this functionality unless the commit needs to be completely reverted.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146200566
  
    @iampeter, could you please have a look so that we can quickly merge this? 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r41617865
  
    --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/TaskManagersHandler.java ---
    @@ -54,10 +58,22 @@ public String handleRequest(Map<String, String> params) throws Exception {
     			ActorGateway jobManager = retriever.getJobManagerGateway();
     
     			if (jobManager != null) {
    -				Future<Object> future = jobManager.ask(JobManagerMessages.getRequestRegisteredTaskManagers(), timeout);
    -				RegisteredTaskManagers taskManagers = (RegisteredTaskManagers) Await.result(future, timeout);
    -
    -				final List<Instance> instances = new ArrayList<Instance>(taskManagers.asJavaCollection());
    +				// whether one task manager's metrics are requested, or all task manager, we
    +				// return them in an array. This avoids unnecessary code complexity.
    +				// If only one task manager is requested, we only fetch one task manager metrics.
    +				final List<Instance> instances = new ArrayList<>();
    +				if (params.containsKey(TASK_MANAGER_ID_KEY)) {
    +					InstanceID instanceID = new InstanceID(StringUtils.hexStringToByte(params.get(TASK_MANAGER_ID_KEY)));
    +					Future<Object> future = jobManager.ask(new JobManagerMessages.RequestTaskManagerInstance(instanceID), timeout);
    +					TaskManagerInstance instance = (TaskManagerInstance) Await.result(future, timeout);
    +					if (instance.instance() != null) {
    --- End diff --
    
    Not sure about the null check. Could check for InstanceNotFound message instead.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146871440
  
    Looks good to merge now.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r44277088
  
    --- Diff: flink-runtime-web/web-dashboard/bower.json ---
    @@ -23,7 +23,8 @@
         "dagre-d3": "~0.4.10",
         "font-awesome": "~4.3.0",
         "moment-duration-format": "~1.3.0",
    -    "qtip2": "~2.2.1"
    +    "qtip2": "~2.2.1",
    +    "highcharts-release": "~4.1.8"
    --- End diff --
    
    Thanks! For now, we'll try to make a release without that page. We can add that back for 0.10.1 and 1.0


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r41612793
  
    --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/TaskManagersHandler.java ---
    @@ -39,6 +42,7 @@
     
     	private final FiniteDuration timeout;
     
    +	private static final String taskManagerIDKey = "taskmanagerid";
    --- End diff --
    
    This is referencing your addition in WebRuntimeMonitor. You can use this constant there as well. I would furthermore use the constant field style a la "TASKMANAGER_ID_KEY".


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146915677
  
    Build passes successfully. Looking forward to seeing this merged. :)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r41618643
  
    --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/TaskManagersHandler.java ---
    @@ -54,10 +58,22 @@ public String handleRequest(Map<String, String> params) throws Exception {
     			ActorGateway jobManager = retriever.getJobManagerGateway();
     
     			if (jobManager != null) {
    -				Future<Object> future = jobManager.ask(JobManagerMessages.getRequestRegisteredTaskManagers(), timeout);
    -				RegisteredTaskManagers taskManagers = (RegisteredTaskManagers) Await.result(future, timeout);
    -
    -				final List<Instance> instances = new ArrayList<Instance>(taskManagers.asJavaCollection());
    +				// whether one task manager's metrics are requested, or all task manager, we
    +				// return them in an array. This avoids unnecessary code complexity.
    +				// If only one task manager is requested, we only fetch one task manager metrics.
    +				final List<Instance> instances = new ArrayList<>();
    +				if (params.containsKey(TASK_MANAGER_ID_KEY)) {
    +					InstanceID instanceID = new InstanceID(StringUtils.hexStringToByte(params.get(TASK_MANAGER_ID_KEY)));
    +					Future<Object> future = jobManager.ask(new JobManagerMessages.RequestTaskManagerInstance(instanceID), timeout);
    +					TaskManagerInstance instance = (TaskManagerInstance) Await.result(future, timeout);
    +					if (instance.instance() != null) {
    --- End diff --
    
    Will fix this.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146439104
  
    @iampeter , thanks for the review. I've addressed all your comments.
    I removed the `No such task ...` thing entirely. It doesn't have a use case anyways. We don't expect the user to break things on the server here. If the task manager doesn't exist, it is handled gracefully by the backend and an empty array is returned.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146440113
  
    I think I broke something while addressing Piotr's concerns. Will debug and push the fixed version again.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146193617
  
    Really cool graphs Sachin :-) Looking forward monitoring my Flink jobs with
    these graphs.
    
    On Wed, Oct 7, 2015 at 3:10 PM, Aljoscha Krettek <no...@github.com>
    wrote:
    
    > Wow, the screenshots look super nice. [image: :smile:]
    >
    > Someone with knowledge about the new web fronted should probably review
    > the code, though.
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/flink/pull/1236#issuecomment-146191633>.
    >



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146193727
  
    Thanks. :)
    Although there's not much to review here. :') It's mostly a rearrangement of existing content, with a major addition in `taskmanager.ctrl.coffee`


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146832468
  
    The initial look is a bit weird, I agree. There is a potential fix, by fetching the usage upto the point when someone clicks on the `metrics` tab, however, that can be quite large if the cluster has been running for a long time. This would require storing the metrics history on the job manager, which will unnecessarily waste memory.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r41619556
  
    --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala ---
    @@ -536,6 +536,11 @@ class JobManager(
             )
           )
     
    +    case RequestTaskManagerInstance(instanceID) =>
    +      sender ! decorateMessage(
    +        TaskManagerInstance(instanceManager.getRegisteredInstanceById(instanceID))
    +      )
    +
    --- End diff --
    
    I think using an Option makes more sense here.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146922016
  
    Merging this after a final test...


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146802623
  
    The screenshots look great. I'll check out your work now :)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146191633
  
    Wow, the screenshots look super nice. :smile:
    
    Someone with knowledge about the new web fronted should probably review the code, though.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r41615077
  
    --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/TaskManagersHandler.java ---
    @@ -39,6 +42,7 @@
     
     	private final FiniteDuration timeout;
     
    +	private static final String taskManagerIDKey = "taskmanagerid";
    --- End diff --
    
    Pushed a fix.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146831304
  
    Tried out your changes it everything worked without quirks. The initial look is a bit odd when the charts haven't loaded yet but this is nothing that should prevent us from merging the PR. I've also made a comment inline.
    
    +1 for merging


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r44280702
  
    --- Diff: flink-runtime-web/web-dashboard/bower.json ---
    @@ -23,7 +23,8 @@
         "dagre-d3": "~0.4.10",
         "font-awesome": "~4.3.0",
         "moment-duration-format": "~1.3.0",
    -    "qtip2": "~2.2.1"
    +    "qtip2": "~2.2.1",
    +    "highcharts-release": "~4.1.8"
    --- End diff --
    
    I'm already working on a change to remove only the charts. Apparently, `d3` would work for us because it is BSD-lincensed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by iampeter <gi...@git.apache.org>.
Github user iampeter commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146333800
  
    @sachingoel0101 @aljoscha looks great, but I would make the following changes if possible:
    
    1. Move the 'livechart' directive to a separate file - `taskmanager.dir.coffee`
    2. Move the Highcharts config in `index.coffee` to either `.run()` in `index.coffee`, or the directive
    3. Remove the 'No such task manager exits' text that blinks when the task manager details are loading
    
    Thanks,
    Piotr
    



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/1236


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1236#discussion_r41617830
  
    --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala ---
    @@ -536,6 +536,11 @@ class JobManager(
             )
           )
     
    +    case RequestTaskManagerInstance(instanceID) =>
    +      sender ! decorateMessage(
    +        TaskManagerInstance(instanceManager.getRegisteredInstanceById(instanceID))
    +      )
    +
    --- End diff --
    
    Wouldn't it be better to return an InstanceNotFound message instead of a null instance?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2730][web-dashboard] Adds cpu and memor...

Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on the pull request:

    https://github.com/apache/flink/pull/1236#issuecomment-146841207
  
    @mxm I modified the code to remove null values. It should be mergeable now.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---