-
Notifications
You must be signed in to change notification settings - Fork 395
[core] Apply hue conf dir and user/group config to gunicorn options #4158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the gunicorn launch command to respect a new HUE_RUN_DIR
environment variable for temporary worker directories, and to conditionally apply Hue’s configured user/group only when GUNICORN_USE_OS_USER
is unset. It also refactors the gunicorn config to pull user
, group
, and worker_tmp_dir
from the local options
dict.
- Add
HUE_RUN_DIR
lookup before falling back toHUE_CONF_DIR
or/tmp
- Only set
options['user']
/['group']
whenGUNICORN_USE_OS_USER
is not defined - Replace direct
conf.SERVER_USER
/SERVER_GROUP
calls in gunicorn config withoptions.get(...)
Comments suppressed due to low confidence (3)
desktop/core/src/desktop/management/commands/rungunicornserver.py:198
- Consider adding automated tests to verify behavior when
GUNICORN_USE_OS_USER
is set versus unset, as well as tests for theworker_tmp_dir
resolution across the three fallbacks.
if not os.environ.get("GUNICORN_USE_OS_USER"):
desktop/core/src/desktop/management/commands/rungunicornserver.py:189
- [nitpick] It may be helpful to update the module docstring or contributing guide to document the new
HUE_RUN_DIR
andGUNICORN_USE_OS_USER
environment variables so users know how to override defaults.
worker_tmp_dir = os.environ.get("HUE_RUN_DIR")
desktop/core/src/desktop/management/commands/rungunicornserver.py:189
- [nitpick] The nested fallback logic for
worker_tmp_dir
could be simplified into a single call toos.environ.get
with defaults, e.g.:os.environ.get('HUE_RUN_DIR', os.environ.get('HUE_CONF_DIR', get_desktop_root('conf') or '/tmp'))
to reduce indentation and improve readability.
worker_tmp_dir = os.environ.get("HUE_RUN_DIR")
@@ -184,10 +184,21 @@ def argprocessing(args=[], options={}): | |||
# Currently gunicorn does not support passphrase suppored SSL Keyfile | |||
# https://github.com/benoitc/gunicorn/issues/2410 | |||
ssl_keyfile = None | |||
worker_tmp_dir = os.environ.get("HUE_CONF_DIR", get_desktop_root("conf")) | |||
|
|||
# HUE_RUN_DIR is set for CDW pods. If unset, fall back to previous behaviour (for base). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Fix or remove comment?
What changes were proposed in this pull request?
Apply hue conf dir and user/group config to gunicorn options
How was this patch tested?
Manual test
Please review Hue Contributing Guide before opening a pull request.