HUE-9490 [jobbrowser] API - Listing Hive query details

Review Request #15472 — Created Sept. 29, 2020 and submitted

ayush.goyal
hue
master
HUE-9490
hue
Amlesh1902, johan, ranade, romain, Sreenath, yingc
commit 827e43a9276e6f007989840dbbbab9042fc02373
Author: ayush.goyal <ayush.goyal@cloudera.com>
Date:   Tue Sep 29 19:37:14 2020 +0530

    HUE-9490 [jobbrowser] API - Listing Hive query details

:100644 100644 e551c4567d e9b3fe153f M	apps/jobbrowser/src/jobbrowser/api2.py
:100644 100644 9511fa096f 980533f386 M	apps/jobbrowser/src/jobbrowser/apis/hive_query_api.py


  • 24
  • 0
  • 0
  • 0
  • 24
Description From Last Updated
If headers still needed, to move up to a class variable romain romain
Do we really need all these headers? Does it work with {}? With only 'Content-Type': 'application/json', Or/and 'X-Requested-With': 'XMLHttpRequest', 'X-Requested-By': ... romain romain
Factor out the DAS url in a new config property in [jobbrowser]? Note: will have to add extra properties for ... romain romain
To move up with the same imports. Actually, we don't use TestCase so to remove and do like class TestHiveQueryApi():, ... romain romain
Here we should test the API (i.e. apps(), app(), what we are implementing in the API), not the model (similar ... romain romain
We can't change this file without breaking all the other Job Browsers (so to workaround in the hive api module) romain romain
if interface == 'queries-hive': .... romain romain
if interface == 'queries-hive': app_id = json.loads(request.body)['queryId'] else: app_id = json.loads(request.POST.get('app_id') romain romain
if interface == 'queries-hive': return JsonResponse(response['app']) else: return JsonResponse(response) romain romain
HEADERS = {'X-Requested-By': 'das'} romain romain
100 currently I guess instead, we can adapt when we take the filtering into account romain romain
As we will add more params (credentials etc..) soon, could we make it a subsection instead of single config params? ... romain romain
DAS --> QUERY_STORE everywhere romain romain
Credentials for query store API. romain romain
URL of Query Store server. romain romain
Let's avoid DAS as the name and keep it more generic. How about query_store like for the DAS db config? romain romain
nit: let's avoid the extra indentation romain romain
response['logs'] = get_api(request.user, interface, cluster=cluster).logs( app_id, app_type, log_name, json.loads(request.GET.get('is_embeddable', 'false').lower()) ) romain romain
app_filters = dict([ (key, value) for _filter in json.loads(request.POST.get('app_filters', '[]')) for key, value in list(_filter.items()) if value ]) romain romain
]) romain romain
extra space romain romain
SERVER --> API server romain romain
ditto romain romain
-2 indentation Space after # SERVER --> API server romain romain
romain
  1. Could you git add the test file too?
    And push to an upstream branch

  2. 
      
ayush.goyal
romain
  1. 
      
  2. If headers still needed, to move up to a class variable

  3. Do we really need all these headers?

    Does it work with {}?

    With only 'Content-Type': 'application/json',

    Or/and 'X-Requested-With': 'XMLHttpRequest',
    'X-Requested-By': 'das',`?

  4. Factor out the DAS url in a new config property in [jobbrowser]?

    Note: will have to add extra properties for the authentication at some point.

  5. To move up with the same imports.

    Actually, we don't use TestCase so to remove and do like class TestHiveQueryApi():, not call to super, like below

    Does it still fail afterwards?

  6. Here we should test the API (i.e. apps(), app(), what we are implementing in the API), not the model (similar to below)

  7. 
      
ayush.goyal
romain
  1. Small changes, can be done on a 3rd commit on top of the previous two, then we can ship and iterate more

  2. apps/jobbrowser/src/jobbrowser/api2.py (Diff revision 3)
     
     

    We can't change this file without breaking all the other Job Browsers (so to workaround in the hive api module)

  3. HEADERS = {'X-Requested-By': 'das'}

  4. 100 currently I guess instead, we can adapt when we take the filtering into account

  5. apps/jobbrowser/src/jobbrowser/conf.py (Diff revision 3)
     
     

    As we will add more params (credentials etc..) soon, could we make it a subsection instead of single config params?

    e.g. like https://github.com/cloudera/hue/blob/master/desktop/libs/metadata/src/metadata/conf.py#L276

  6. 
      
romain
  1. 
      
  2. apps/jobbrowser/src/jobbrowser/api2.py (Diff revision 3)
     
     

    if interface == 'queries-hive':
    ....

  3. apps/jobbrowser/src/jobbrowser/api2.py (Diff revision 3)
     
     

    if interface == 'queries-hive':
    app_id = json.loads(request.body)['queryId']
    else:
    app_id = json.loads(request.POST.get('app_id')

  4. apps/jobbrowser/src/jobbrowser/api2.py (Diff revision 3)
     
     

    if interface == 'queries-hive':
    return JsonResponse(response['app'])
    else:
    return JsonResponse(response)

  5. 
      
ayush.goyal
ayush.goyal
romain
  1. Almost!

  2. apps/jobbrowser/src/jobbrowser/conf.py (Diff revision 4)
     
     

    DAS --> QUERY_STORE

    everywhere

  3. apps/jobbrowser/src/jobbrowser/conf.py (Diff revision 4)
     
     

    Credentials for query store API.

  4. apps/jobbrowser/src/jobbrowser/conf.py (Diff revision 4)
     
     

    URL of Query Store server.

  5. Let's avoid DAS as the name and keep it more generic.

    How about query_store like for the DAS db config?

  6. desktop/conf/pseudo-distributed.ini.tmpl (Diff revision 4)
     
     
     

    nit: let's avoid the extra indentation

  7. 
      
romain
  1. 
      
  2. apps/jobbrowser/src/jobbrowser/api2.py (Diff revision 5)
     
     
     

    response['logs'] = get_api(request.user, interface, cluster=cluster).logs(
    app_id, app_type, log_name, json.loads(request.GET.get('is_embeddable', 'false').lower())
    )

  3. apps/jobbrowser/src/jobbrowser/api2.py (Diff revision 5)
     
     
     

    app_filters = dict([
    (key, value) for _filter in json.loads(request.POST.get('app_filters', '[]'))
    for key, value in list(_filter.items()) if value
    ])

  4. 
      
ayush.goyal
romain
  1. 
      
  2. apps/jobbrowser/src/jobbrowser/conf.py (Diff revision 6)
     
     

    SERVER --> API server

  3. desktop/conf.dist/hue.ini (Diff revision 6)
     
     
  4. desktop/conf/pseudo-distributed.ini.tmpl (Diff revision 6)
     
     
     

    -2 indentation

    Space after #

    SERVER --> API server

  5. 
      
ayush.goyal
ayush.goyal
Review request changed

Status: Closed (submitted)

Loading...