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

Review Request #15518 — Created Oct. 22, 2020 and submitted Oct. 23, 2020, 5:28 a.m.

Information
Owner:	ayush.goyal
Repository:	hue
Branch:	master
Bugs:	HUE-9507
Depends On:
Reviewers
Groups:	hue
People:	Amlesh1902, johan, ranade, romain, Sreenath, yingc

Description

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

Testing Done

Issues

5
0
0
0
All Issues: 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	Oct. 22, 2020, 6:30 a.m.
Double (())?	romain	Oct. 22, 2020, 6:15 a.m.
fileName --> file_name or no need of variable	romain	Oct. 22, 2020, 6:15 a.m.
Nit: use single quotes ' (more consistent and even avoid backslashes)	romain	Oct. 22, 2020, 6:15 a.m.
To combine all together	romain	Oct. 22, 2020, 6:15 a.m.

Fix it!

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.

apps/jobbrowser/src/jobbrowser/api2.py (Diff revision 1)
Show all issues
```
Double (())?
```
apps/jobbrowser/src/jobbrowser/api2.py (Diff revision 1)
Show all issues
```
fileName --> file_name or no need of variable
```

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

Show all issues

Nit: use single quotes '
(more consistent and even avoid backslashes)

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

Show all issues

To combine all together

ayush.goyal Oct. 22, 2020, 8:47 a.m.

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.

romain Oct. 22, 2020, 9:27 a.m.

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.

Fix it!

Show all issues

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.

romain Oct. 22, 2020, 8:20 a.m.

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

Sreenath Oct. 22, 2020, 9:06 a.m.

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.

Diff:

Revision 2 (+19 -1)

Show changes

	apps/jobbrowser/src/jobbrowser/api2.py
	apps/jobbrowser/src/jobbrowser/urls.py

You have a pending review.

Review Board 3.0.18

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

Diff:

Status: Closed (submitted)