-
-
desktop/core/src/desktop/models.py (Diff revision 1) Not sure which conf this is pointing to but you can better add on top:
from filebrowser.conf import REMOTE_STORAGE_HOME
and then:
if REMOTE_STORAGE_HOME.get():
HUE-9455 [filebrowser] File system user home directory is wrong in S3 only configuration
Review Request #15403 — Created Aug. 27, 2020 and submitted
Information | |
---|---|
ayush.goyal | |
hue | |
master | |
HUE-9455 | |
Reviewers | |
hue | |
Amlesh1902, johan, ranade, romain, Sreenath, yingc |
commit bdef2458e16f763710197234153364f7015c35fe Author: agl29 <ayushkr.goyal.eee15@itbhu.ac.in> Date: Thu Aug 27 19:01:14 2020 +0530 HUE-9455 File system user home directory is wrong in S3 only configuration :100644 100644 67a9b47fa2 79945e7402 M apps/filebrowser/src/filebrowser/conf.py :100644 100644 17e2ef3b52 b4f72981db M desktop/core/src/desktop/models.py
- 12
- 0
- 0
- 0
- 12
Description | From | Last Updated |
---|---|---|
Not sure which conf this is pointing to but you can better add on top: from filebrowser.conf import REMOTE_STORAGE_HOME and ... |
|
|
Here this is the home directory aka on the home lage that contains the user documents (/home), so this is ... |
|
|
nit, add an example? (also to update in both inis): Optionally set this if you want a different home directory ... |
|
|
If it gets too long, feel free to add a variable each time? e.g. home_path = (REMOTE_STORAGE_HOME.get() if REMOTE_STORAGE_HOME.get() else ... |
|
|
ditto |
|
|
remote_storage_home=s3a://gethue ? |
|
|
import to remove too |
|
|
Wouldn't it be better to move the logic into `get_home_directory()` instead? |
|
|
Could you also add something similar to https://github.com/cloudera/hue/blob/master/desktop/core/src/desktop/templates/global_js_constants.mako#L121 REMOTE_STORAGE_HOME.get() if hasattr(REMOTE_STORAGE_HOME, 'get') and REMOTE_STORAGE_HOME.get() else user.get_home_directory() to avoid the problem ... |
|
|
For the record, if it repeats a lot, better to have the setting name = None, e.g. https://github.com/cloudera/hue/blob/master/desktop/libs/notebook/src/notebook/connectors/hiveserver2.py#L68 Even better, ... |
|
|
and then: home_path = remote_home_storage if remote_home_storage else self.user.get_home_directory().encode('utf-8') .... |
|
|
Move to line 1918? (surrounded by new lines) (Thay way it is scoped only in the function, not global) |
|
-
-
desktop/core/src/desktop/models.py (Diff revision 2) Here this is the home directory aka on the home lage that contains the user documents (/home), so this is unrelated to File Browser. Could you update the specific storage filesystem home dir in File Browser instead?
cf. the jira, the idea is to have for example a default landing S3 bucket
And also fix the refresh icon of the left assist file browser
-
Nice!
And any chance to add a small test?
-
apps/filebrowser/src/filebrowser/conf.py (Diff revision 3) nit, add an example? (also to update in both inis):
Optionally set this if you want a different home directory path. e.g. s3a://gethue
-
desktop/core/src/desktop/models.py (Diff revision 3) If it gets too long, feel free to add a variable each time?
e.g.
home_path = (REMOTE_STORAGE_HOME.get() if REMOTE_STORAGE_HOME.get() else 'S3A://').encode('utf-8')
Also note the extra encoding. Does it work when specifying a bucket (and having only S3 configured?)
remote_storage_home=s3a://demo-gethue
Change Summary:
Reopen -> earlier also working for all the users (same custom_home) but there are some other corrections. (left assist panel's home button not working for s3 should i have to fix that also, now?)
Diff: |
Revision 7 (+3 -3) |
---|
-
-
desktop/core/src/desktop/templates/global_js_constants.mako (Diff revision 7) Wouldn't it be better to move the logic into `get_home_directory()` instead?
-
-
desktop/core/src/desktop/templates/global_js_constants.mako (Diff revision 7) Could you also add something similar to
https://github.com/cloudera/hue/blob/master/desktop/core/src/desktop/templates/global_js_constants.mako#L121
REMOTE_STORAGE_HOME.get() if hasattr(REMOTE_STORAGE_HOME, 'get') and REMOTE_STORAGE_HOME.get() else user.get_home_directory()
to avoid the problem when the app is blacklister
-
Notes: - Please use a fresh review next time (after the first commit is pushed, we should close the review and use fresh ones for the follow-ups, it is much clearer) - To test it with file browser out, in the [desktop] section of the ini, app_blacklist=filebrowser: e.g. [desktop] app_blacklist=filebrowser
-
desktop/core/src/desktop/models.py (Diff revision 8) For the record, if it repeats a lot, better to have the
setting name = None
, e.g. https://github.com/cloudera/hue/blob/master/desktop/libs/notebook/src/notebook/connectors/hiveserver2.py#L68Even better, maybe just extrac the value once on top:
remote_home_storage =
REMOTE_STORAGE_HOME.get() if hasattr(REMOTE_STORAGE_HOME, 'get') and REMOTE_STORAGE_HOME.get() else None -
desktop/core/src/desktop/models.py (Diff revision 8) and then:
home_path = remote_home_storage if remote_home_storage else self.user.get_home_directory().encode('utf-8')
....
-
desktop/core/src/desktop/models.py (Diff revision 8) Note (for in the future): at some point we should have a get_home() for each fs)
-
-
desktop/core/src/desktop/models.py (Diff revision 9) Move to line 1918? (surrounded by new lines)
(Thay way it is scoped only in the function, not global)