HUE-9507 [querybrowser] API - Proxy log download API

Review Request #15518 — Created Oct. 22, 2020 and submitted

ayush.goyal
hue
master
HUE-9507
hue
Amlesh1902, johan, ranade, romain, Sreenath, yingc
commit c90c28e5626a7e7c3e24c09267a23b8aafb47b85
Author: ayush.goyal <ayush.goyal@cloudera.com>
Date:   Thu Oct 22 18:04:30 2020 +0530

    HUE-9507 [querybrowser] API - Proxy log download API

:100644 100644 0f07207cfb d3c5cc890a M	apps/jobbrowser/src/jobbrowser/api2.py
:100644 100644 8aacefd485 ef5bca63ed M	apps/jobbrowser/src/jobbrowser/urls.py


  • 5
  • 0
  • 0
  • 0
  • 5
Description From Last Updated
Ideally as soon as download is clicked the zip stream must start downloading, on testing this implementation the UI didn’t ... Sreenath Sreenath
Double (())? romain romain
fileName --> file_name or no need of variable romain romain
Nit: use single quotes ' (more consistent and even avoid backslashes) romain romain
To combine all together romain romain
romain
  1. Nice!

    Interesting that no need of special headers.
    Nit: we might want to refactor
    client = HttpClient(QUERY_STORE.SERVER_URL.get())
    into a _get_client() util at some point to avoid the duplication (especially when we add the kerberos auth)

    Not simple to add real tests, so good without.

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

    fileName --> file_name or no need of variable

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

    Nit: use single quotes '

    (more consistent and even avoid backslashes)

  5. apps/jobbrowser/src/jobbrowser/urls.py (Diff revision 1)
     
     
     
     
     
     
     

    To combine all together

    1. not combining because as we want proxied download api always (i.e. in other versions also) and the other proxies will be removed in other versions. So only need to delete the other one not the download one.

    2. I see but so we need a feature flag too, as we are not going to change the code to flip between versions. The feature flag will say if we proxy or not proxy.

  6. 
      
Sreenath
  1. 
      
  2. Ideally as soon as download is clicked the zip stream must start downloading, on testing this implementation the UI didn’t react till the zip was created.

    1. Good point, let's get the v1 in, then we could quick poke why the file content is not seen as a generator and streamed back right away, or see for more a quick UI feedback action

    2. If the file is streamed the download must start immediately. This looked like the file is getting stored in memory before being send to the client side.

  3. 
      
ayush.goyal
ayush.goyal
Review request changed

Status: Closed (submitted)

Loading...