HUE-7812 [core] Metrics and Threads page don't get refreshed

Review Request #12268 - Created Dec. 21, 2017 and submitted

Roohi Syeda
hue
master
HUE-7812
hue
enricoberti, jgauthier, johan, ranade, romain, weixia, yingc
commit 5dc57601df007191eb9113c933bc5deac42781e1
Author: Roohi <roohisyeda@cloudera.com>
Date:   Thu Dec 21 16:26:15 2017 -0800

    HUE-7812 [core] Metrics and Threads page don't get refreshed

:100644 100644 2d06492508... 9abbe3ea16... M	desktop/core/src/desktop/templates/hue.mako
:100644 100644 b4994b76d2... 71c7643ed8... M	desktop/core/src/desktop/templates/metrics.mako

Manual testing on chrome

  • 0
  • 0
  • 12
  • 0
  • 12
Description From Last Updated
  1. 
      
  2. I would recommend one commit with only

    'threads'

    overall

    as we would need to fix in 5.14.1 which only has threads,

    then thread metrics as a separate commit, then the real fetch of the thread data like explained above in another commit too

    (here there is no point in doing above if we keep the skip cache, which refreshes the full page)

  3. ko.observableArray() bit cleaner

  4. self.fetchMetrics = function() {
    self.apiHelper.simpleGet('/desktop/metrics/', {}, function(data) {
    // usual check for data.code cf. indexes or console.log(data) content

    then if == 0
    self.metrics(data.metric);
    });
    }

    then in hue.mako you could see on how to get the model and call loadapp(); model.fetchMetrics()

  5. 
      
  1. 
      
  2. Nit: No need for "var successCallback"

    you could do simpleGet(..., { successCallback: self.threads });

  3. desktop/core/src/desktop/templates/threads.mako (Diff revision 3)
     
     
     
     
     
     
     

    indentation

  4. When reload is set to false this function won't do anything, so better to drop the reload flag here and make not call the function in the case where reload would be false.

  5. 
      
  1. Nice one!

  2. Is 'threads' commit already pushed?

    (as we would need to backport it, so easier if added in a small commit before)

    nit: remove space before comma

  3. nit: keep new line?

  4. $('#threadsComponents')[0] instead of getElementById

    should do the same?

    and do we really need the ko.cleanNode?

  5. 
      
  1. Nice!

    And refresh buttons in another commit?

  2. nit: extra space after , app

  3. Could we put it on 3 lines?

  4. 
      
Review request changed

Status: Closed (submitted)

Loading...