HUE-9490 [jobbrowser] API - Listing Hive query details
Review Request #15472 — Created Sept. 29, 2020 and submitted
Information | |
---|---|
ayush.goyal | |
hue | |
master | |
HUE-9490 | |
Reviewers | |
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 |
|
|
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': ... |
|
|
Factor out the DAS url in a new config property in [jobbrowser]? Note: will have to add extra properties for ... |
|
|
To move up with the same imports. Actually, we don't use TestCase so to remove and do like class TestHiveQueryApi():, ... |
|
|
Here we should test the API (i.e. apps(), app(), what we are implementing in the API), not the model (similar ... |
|
|
We can't change this file without breaking all the other Job Browsers (so to workaround in the hive api module) |
|
|
if interface == 'queries-hive': .... |
|
|
if interface == 'queries-hive': app_id = json.loads(request.body)['queryId'] else: app_id = json.loads(request.POST.get('app_id') |
|
|
if interface == 'queries-hive': return JsonResponse(response['app']) else: return JsonResponse(response) |
|
|
HEADERS = {'X-Requested-By': 'das'} |
|
|
100 currently I guess instead, we can adapt when we take the filtering into account |
|
|
As we will add more params (credentials etc..) soon, could we make it a subsection instead of single config params? ... |
|
|
DAS --> QUERY_STORE everywhere |
|
|
Credentials for query store API. |
|
|
URL of Query Store server. |
|
|
Let's avoid DAS as the name and keep it more generic. How about query_store like for the DAS db config? |
|
|
nit: let's avoid the extra indentation |
|
|
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()) ) |
|
|
app_filters = dict([ (key, value) for _filter in json.loads(request.POST.get('app_filters', '[]')) for key, value in list(_filter.items()) if value ]) |
|
|
]) |
|
|
extra space |
|
|
SERVER --> API server |
|
|
ditto |
|
|
-2 indentation Space after # SERVER --> API server |
|
-
-
apps/jobbrowser/src/jobbrowser/apis/hive_query_api.py (Diff revision 2) If headers still needed, to move up to a class variable
-
apps/jobbrowser/src/jobbrowser/apis/hive_query_api.py (Diff revision 2) 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',`? -
apps/jobbrowser/src/jobbrowser/apis/hive_query_api.py (Diff revision 2) 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.
-
apps/jobbrowser/src/jobbrowser/apis/hive_query_api_tests.py (Diff revision 2) 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 belowDoes it still fail afterwards?
-
apps/jobbrowser/src/jobbrowser/apis/hive_query_api_tests.py (Diff revision 2) Here we should test the API (i.e. apps(), app(), what we are implementing in the API), not the model (similar to below)
-
Small changes, can be done on a 3rd commit on top of the previous two, then we can ship and iterate more
-
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)
-
apps/jobbrowser/src/jobbrowser/apis/hive_query_api.py (Diff revision 3) HEADERS = {'X-Requested-By': 'das'}
-
apps/jobbrowser/src/jobbrowser/apis/hive_query_api.py (Diff revision 3) 100 currently I guess instead, we can adapt when we take the filtering into account
-
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
-
-
-
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') -
apps/jobbrowser/src/jobbrowser/api2.py (Diff revision 3) if interface == 'queries-hive':
return JsonResponse(response['app'])
else:
return JsonResponse(response)
-
Almost!
-
-
-
-
desktop/conf/pseudo-distributed.ini.tmpl (Diff revision 4) Let's avoid DAS as the name and keep it more generic.
How about
query_store
like for the DAS db config? -
-
-
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())
) -
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
])
-
-
-
-
-
-
desktop/conf/pseudo-distributed.ini.tmpl (Diff revision 6) -2 indentation
Space after #
SERVER --> API server