HUE-9492 [query browser] Text search, date filtering and pagination and unittest

Review Request #15493 — Created Oct. 6, 2020 and submitted

ayush.goyal
hue
master
HUE-9492
hue
Amlesh1902, johan, ranade, romain, Sreenath, yingc

commit e036b7220439a55fc88a37fd52aaabf4708fb6de (HEAD -> HUE-9492)
Author: ayush.goyal ayush.goyal@cloudera.com
Date: Thu Oct 8 16:15:56 2020 +0530

HUE-9492 [query browser] adding unittest for Api query listing, pagination, query_count

commit 312d93fc5e6c02c9608aa5b6ed264e0a1e750d31
Author: ayush.goyal ayush.goyal@cloudera.com
Date: Thu Oct 8 01:56:21 2020 +0530

HUE-9492 [query browser] Text search, date filtering and pagination


  • 10
  • 0
  • 0
  • 0
  • 10
Description From Last Updated
Please see for moving all the filtering to the ORM (that way the work gets pushed down at the DB ... romain romain
No paranthesis needed romain romain
help_get_queries --> _get_queries (convention for private methods in Python) romain romain
Actually, seems like it would make more sense to remove the queries argument and instead of inside: queries = HiveQuery.objects.using('query') romain romain
if filter['text']: romain romain
We should avoid any DB specific properties (the whole idea of the ORM is to be compatible with DBs other ... romain romain
Could you add a test for the pagination? def test_search_pagination(self): with patch('jobbrowser.apis.hive_query_api.HiveQueryClient.get_queries') as get_queries: ... get_queries.return_value = [{.....}, {.....}] romain romain
--> test_search_pagination romain romain
Move to TestHiveClient()? Testing the API means whe nwe call the API, e.g. self.client.post("/jobbrowser/api/jobs/queries-hive", content_type='applecation/json', data=filters) romain romain
nit: could move it up above line 204 and then below we can reuse query1.query_id = 'query_id romain romain
romain
  1. Add a unit test too?

  2. apps/jobbrowser/src/jobbrowser/apis/hive_query_api.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Please see for moving all the filtering to the ORM (that way the work gets pushed down at the DB level instead of the API level)

    e.g. https://github.com/cloudera/hue/blob/master/desktop/core/src/desktop/api.py

  3. No paranthesis needed

  4. 
      
ayush.goyal
romain
  1. Nice one!

    Minor comments below and add minimal unit tests?

  2. help_get_queries --> _get_queries

    (convention for private methods in Python)

  3. Actually,

    seems like it would make more sense to remove the queries argument and instead of inside:

    queries = HiveQuery.objects.using('query')

  4. if filter['text']:
  5. 
      
romain
  1. 
      
  2. desktop/core/src/desktop/settings.py (Diff revision 2)
     
     

    We should avoid any DB specific properties (the whole idea of the ORM is to be compatible with DBs other that psql). Regular prefix search is fine for v1 (and free text search require usability/perf tests/multi db testing... which is pretty much a lot more work to guarantee a stable experience)

  3. 
      
ayush.goyal
ayush.goyal
ayush.goyal
romain
  1. Nice!

    Nit, bit more explicit: HUE-9492 [query browser] Text search, date filtering and pagination

  2. Could you add a test for the pagination?
    
    def test_search_pagination(self):
          with patch('jobbrowser.apis.hive_query_api.HiveQueryClient.get_queries') as get_queries:
          
          ...
          
          get_queries.return_value = [{.....}, {.....}]
  3. 
      
ayush.goyal
ayush.goyal
romain
  1. 
      
  2. --> test_search_pagination

    1. did you mean that change the name of test_apps_empty(pagination is not tested here) to test_search_pagination?
      if yes then we are using test_search_pagination in TestHiveClient() as per your below comment?

    2. what i did pagination test also included in test_apps_empty and then rename it to test_search_pagination

  3. Move to TestHiveClient()?

    Testing the API means whe nwe call the API, e.g. self.client.post("/jobbrowser/api/jobs/queries-hive", content_type='applecation/json', data=filters)

  4. nit: could move it up above line 204
    and then below we can reuse

    query1.query_id = 'query_id

  5. 
      
ayush.goyal
ayush.goyal
Review request changed

Status: Closed (submitted)

Loading...