HUE-5439 [assist] Show database comment as tooltip in editors and add a link to the Metastore DB page in the information popup

Review Request #9725 - Created March 6, 2017 and submitted

Adrian Yavorskyy
hue
master
hue
enricoberti, erickt, johan, romain
commit 6322242a871e32dc7f13945d5e691f949fd0a96e
Author: Adrian Yavorskyy <ayav@softserveinc.com>
Date:   Mon Mar 6 17:40:24 2017 +0200

    HUE-5439 [assist] Show database comment as tooltip in editors and add a link to the Metastore DB page in the information popup

:100644 100644 1dce62b... c843294... M	desktop/core/src/desktop/static/desktop/js/assist/assistDbSource.js
:100644 100644 ee8b614... 7f9d2ed... M	desktop/core/src/desktop/templates/assist.mako
:100644 100644 6729794... 638876d... M	desktop/core/src/desktop/templates/sql_context_popover.mako

Manual in Chrome

  • 1
  • 0
  • 6
  • 0
  • 7
Description From Last Updated
Johan, need to add comments to the helper API? Romain Rigaux
  1. 
      
  2. No need for deferred here, just put line 215-234 in success callback

    1. As I found, ApiHelper has a method loadDatabases, but it's only uploading the names of a databases as a list, that's all. Maybe I am missing something, please correct if I am wrong.

  3. Comment should already be available in the entries elsewhere, shouldn't be a need to fetch the databases again.

    1. I've found comment for a database only in metsatore app, in overview page for database(e.g. http://localhost/metastore/tables/default). The commments are uploaded using $.getJSON(https://github.com/cloudera/hue/blob/master/apps/metastore/src/metastore/static/metastore/js/metastore.ko.js#L187).

  4. Change above line to isTable || isDatabase, pass type in pubsub, i.e. just one subscription for open.in.metastore

    1. I changed the line above, but still need subscription in TableAndColumnContextTabs and DatabaseContextTabs. It's working, but as I understand you meant to leave only one subscription. If I am leaving one subscription, do I have to move subscription to GenericTabContents? Am I missing something?

  5. 
      
  1. 
      
  2. Johan, need to add comments to the helper API?

    1. I think the best approach is to add it to the response for notebook/api/autocomplete/ that way we get them when fetching all databases and we don't have to make separate calls for each DB. We can also show the comment in the metastore database list like we do for tables as well as in the autocomplete dropdown. Can you adjust Romain?

    2. For DB, unfortunately Hive API wise there is not way to bulk fetch the comments when listing the databases (only when fetching one by one)

    3. So, what should be done with this issue? Is there anything that I can do ?

    4. Sorry for not getting back to you on this one. You can move this part to the ApiHelper instead and drop the deferred then it should be good.

    5. Johan, I am trying to do what you suggested: I moved this part in ApiHelper.prototype.loadDatabases( https://github.com/cloudera/hue/blob/master/desktop/core/src/desktop/static/desktop/js/apiHelper.js#L893), but without deferred I can't get it working. Maybe I should create a new method in ApiHelper for loading comments using fetchAssistData(I have no idea if it possible)? Or what else could be done?

    6. I meant a new function in ApiHelper instead. We shouldn't fetch each comment when loading all the databases but rather on demand. So your original solution was good, except that you don't need the deferred, just move the ajax call to the ApiHelper and perform the actions in the success callback.

  3. 
      
  1. We can't fetch all the comments for all the databases in one go, it should rather be done on demand for a specific database.

    1. Sorry for disturbing you so much with this review, but as I see here (https://github.com/cloudera/hue/blob/master/desktop/core/src/desktop/static/desktop/js/assist/assistDbSource.js#L199) a data about all databases is being gathered to be shown in assist. So, my idea was to upload comments and add them to a creation of new AssistDbEntry. I can change function in ApiHelper to receive comment only for a specific database, but as I assume later this function should be called in a cycle to receive all comments and use them in updateDatabases. Should it be like this or I am thinking in wrong direction ?

    2. As you can see it just creates the objects representing the databases (AssistDbEntry), it doesn't actually fetch anything there. It will only fetch the tables for a database when the AssistDbEntry is opened.

      We shouldn't fetch the comment for each DB as that won't perform that well. You can easily show (and fetch) it in the context popover when it's shown, but as a tooltip in the assist might be a bit nasty to implement.

    3. I understand it now, thanks very much for explanation! Would it be ok if I showed a comment as a tooltip in header of the context popover where the name of a database is ?

  2. 
      
  1. 
      
  2. The comment can be quite long and formatted so it's better to put it in the details panel instead of the title.

    See the table details for an example

    1. Just clarifying: I need to create a new tab 'Details' in popver for databases and put comment there(as in tables), yes ? Right now there is only 'Tags' tab for databases in popover.

    2. Or maybe better rename existing tab to 'Details' and display there tags and comment(in order not to create extra tabs) ?

    3. +1 on renaming the existing one. Thanks!

  3. 
      
  1. Just need to update the commit message then good to go! Thanks!

    1. I have added a patch to a jira with updated commit message. Is that enough?

    2. Hi Adrian, the patch doesn't apply on master. Can you please pull, rebase and re-send the updated patch? Thanks!

    3. Hi Enrico. Sure, I will do that.

    4. I have updated the patch. Could you please try one more time?

    5. Works! Thanks!

  2. 
      
Review request changed

Status: Closed (submitted)

Loading...