- Download Diff

Summary:

HUE-9478 [api] Job browser proxy

Review Request #15448 — Created Sept. 17, 2020 and submitted Oct. 6, 2020, 8:12 a.m.

Information
Owner:	Sreenath
Repository:	hue
Branch:	master
Bugs:	HUE-9478
Depends On:
Reviewers
Groups:	hue
People:	johan, romain

Description

commit 140117c50ca3057fb3e0da680c289eeba87689e6
Author: sreenaths <sree@apache.org>
Date:   Fri Sep 18 00:38:28 2020 +0530

    HUE-9478. [api] Job browser proxy

:100644 100644 f31fd37b57 e9e284dc32 M	apps/jobbrowser/src/jobbrowser/conf.py
:100644 100644 6fc44f9088 d4c3d74185 M	apps/jobbrowser/src/jobbrowser/urls.py
:100644 100644 3294fd1d14 a5af2901e5 M	desktop/conf.dist/hue.ini
:100644 100644 3dca8321b2 184d84db5a M	desktop/conf/pseudo-distributed.ini.tmpl
:100644 100644 fbf0b5cb46 74351f6a64 M	desktop/core/src/desktop/settings.py

Testing Done

Manually tested, the proxy is created when configuration is set and requests are getting proxied.

Issues

0
0
10
0
All Issues: 10

Description	From	Last Updated
Avoid the new line?	romain	Sept. 22, 2020, 5:47 a.m.
upstreamURL --> upstream_url (Python standard case) (also actually probably no need to have the variable)	romain	Sept. 22, 2020, 5:48 a.m.
If we go this way, would need to check the license and add it to desktop/ext-py and requirements.txt	romain	Oct. 6, 2020, 5:15 a.m.
If not enabled, we should not load the app	romain	Oct. 6, 2020, 5:15 a.m.
To combine	romain	Oct. 6, 2020, 5:56 a.m.
Combine + move above other import on line 26	romain	Oct. 6, 2020, 5:56 a.m.
Move to bottom of file?	romain	Oct. 6, 2020, 5:56 a.m.
One more new line	romain	Oct. 6, 2020, 5:56 a.m.
nit: proxy --> query-proxy?	romain	Oct. 6, 2020, 5:56 a.m.
PS: `content_type='application/json; charset=UTF-8'` can be moved down the function as hardcoded, and later can be auto detected based on the ...	romain	Oct. 6, 2020, 6:16 a.m.

For this one, how do we handle the authentication?

apps/jobbrowser/src/jobbrowser/urls.py (Diff revision 1)
Show all issues
```
Avoid the new line?
```

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

Show all issues

upstreamURL --> upstream_url
(Python standard case)
(also actually probably no need to have the variable)

desktop/core/src/desktop/settings.py (Diff revision 1)

Show all issues

If we go this way, would need to check the license and add it to desktop/ext-py and requirements.txt

desktop/core/src/desktop/settings.py (Diff revision 1)
Show all issues
```
If not enabled, we should not load the app
```

Change Summary:

Moved to an HttpClient based implementation

Diff:

Revision 2 (+22 -5)

Show changes

	apps/jobbrowser/src/jobbrowser/api2.py
	apps/jobbrowser/src/jobbrowser/conf.py
	apps/jobbrowser/src/jobbrowser/urls.py
	desktop/conf/pseudo-distributed.ini.tmpl
	desktop/conf.dist/hue.ini

Ship it!

Nice!

And we can easily add the Kerberos auth flag in a follow-up

apps/jobbrowser/src/jobbrowser/api2.py (Diff revision 2)
Show all issues
```
To combine
```
apps/jobbrowser/src/jobbrowser/api2.py (Diff revision 2)
Show all issues
```
Combine + move above other import on line 26
```

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

Show all issues

Move to bottom of file?

apps/jobbrowser/src/jobbrowser/urls.py (Diff revision 2)
Show all issues
```
One more new line
```
apps/jobbrowser/src/jobbrowser/urls.py (Diff revision 2)
Show all issues
```
nit: proxy --> query-proxy?
```

Change Summary:

Addressed review comments

Diff:

Revision 3 (+25 -9)

Show changes

	apps/jobbrowser/src/jobbrowser/api2.py
	apps/jobbrowser/src/jobbrowser/conf.py
	apps/jobbrowser/src/jobbrowser/urls.py
	desktop/conf/pseudo-distributed.ini.tmpl
	desktop/conf.dist/hue.ini

Diff:

Revision 4 (+26 -8)

Show changes

	apps/jobbrowser/src/jobbrowser/api2.py
	apps/jobbrowser/src/jobbrowser/conf.py
	apps/jobbrowser/src/jobbrowser/urls.py
	desktop/conf/pseudo-distributed.ini.tmpl
	desktop/conf.dist/hue.ini

Ship it!

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

Show all issues

PS: `content_type='application/json; charset=UTF-8'` can be moved down the function as hardcoded, and later can be auto detected based on the content type of the `request` headers
https://docs.djangoproject.com/en/dev/ref/request-response/#django.http.HttpRequest.META

Change Summary:

AAddressing review comments

Diff:

Revision 5 (+27 -8)

Show changes

	apps/jobbrowser/src/jobbrowser/api2.py
	apps/jobbrowser/src/jobbrowser/conf.py
	apps/jobbrowser/src/jobbrowser/urls.py
	desktop/conf/pseudo-distributed.ini.tmpl
	desktop/conf.dist/hue.ini

Diff:

Revision 6 (+27 -8)

Show changes

	apps/jobbrowser/src/jobbrowser/api2.py
	apps/jobbrowser/src/jobbrowser/conf.py
	apps/jobbrowser/src/jobbrowser/urls.py
	desktop/conf/pseudo-distributed.ini.tmpl
	desktop/conf.dist/hue.ini

You have a pending review.

Review Board 3.0.18

HUE-9478 [api] Job browser proxy

Change Summary:

Diff:

Change Summary:

Diff:

Diff:

Change Summary:

Diff:

Diff:

Status: Closed (submitted)