-
Notifications
You must be signed in to change notification settings - Fork 321
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
Allow the usage of gunicorn.conf.py config file in gunicorn applications #684
base: master
Are you sure you want to change the base?
Conversation
[test-all] |
[test-all] |
bf832ff
to
5f8a345
Compare
[test-all] |
5f8a345
to
41cba37
Compare
[test-all] |
[test] |
41cba37
to
a4371c6
Compare
[test] |
a4371c6
to
1c03553
Compare
[test] |
1 similar comment
[test] |
6f79ce0
to
1c03553
Compare
[test] |
1 similar comment
[test] |
db6436c
to
d212002
Compare
[test] |
d212002
to
3e78560
Compare
[test-all] |
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.
The diff looks good to me, in the tests you can see the configfile is used.
Should this test be only added to those running on Python 3? I see there's a test failure on the newly added tests in RHEL8-2.7 container but not sure how it is related.
I assume openshift tests are expected to fail outside of the internal network?
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.
One nitpick and one last fix for Python 2 and I think we are ready. Thank you!
src/s2i/bin/run
Outdated
if is_gunicorn_installed && [[ -f "gunicorn.conf.py" ]]; then | ||
ret=$(python -c 'import gunicorn | ||
ver = gunicorn.version_info | ||
print(0) if ver[0]>=21 or (ver[0] == 20 and ver[1] >= 1) else print(1)') |
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.
This can be a little bit shorter:
print(0) if ver[0]>=21 or (ver[0] == 20 and ver[1] >= 1) else print(1)') | |
print(0 if ver[0]>=21 or (ver[0] == 20 and ver[1] >= 1) else 1)') |
test/run
Outdated
@@ -6,7 +6,7 @@ | |||
# IMAGE_NAME specifies a name of the candidate image used for testing. | |||
# The image has to be available before this script is executed. | |||
# | |||
declare -a COMMON_WEB_APPS=({gunicorn-config-different-port,gunicorn-different-port,django-different-port,standalone,setup,setup-requirements,django,numpy,app-home,locale,pipenv,pipenv-and-micropipenv-should-fail,app-module,pyuwsgi-pipenv{% if spec.version.startswith("3.") %},micropipenv,standalone-custom-pypi-index{% endif %}}-test-app) | |||
declare -a COMMON_WEB_APPS=({gunicorn-config-different-port,gunicorn-different-port,gunicorn-python-configfile-different-port,django-different-port,standalone,setup,setup-requirements,django,numpy,app-home,locale,pipenv,pipenv-and-micropipenv-should-fail,app-module,pyuwsgi-pipenv{% if spec.version.startswith("3.") %},micropipenv,standalone-custom-pypi-index{% endif %}}-test-app) |
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.
Because the test requires gunicorn 20.1+ it will never work with Python 2 because the last gunicorn compatible with Python 2 is version 19.
Therefore we need to move the test to the section for python-3-only.
declare -a COMMON_WEB_APPS=({gunicorn-config-different-port,gunicorn-different-port,gunicorn-python-configfile-different-port,django-different-port,standalone,setup,setup-requirements,django,numpy,app-home,locale,pipenv,pipenv-and-micropipenv-should-fail,app-module,pyuwsgi-pipenv{% if spec.version.startswith("3.") %},micropipenv,standalone-custom-pypi-index{% endif %}}-test-app) | |
declare -a COMMON_WEB_APPS=({gunicorn-config-different-port,gunicorn-different-port,django-different-port,standalone,setup,setup-requirements,django,numpy,app-home,locale,pipenv,pipenv-and-micropipenv-should-fail,app-module,pyuwsgi-pipenv{% if spec.version.startswith("3.") %},micropipenv,standalone-custom-pypi-index,gunicorn-python-configfile-different-port{% endif %}}-test-app) |
3e78560
to
443f6fe
Compare
[test] |
1 similar comment
[test] |
test/run
Outdated
@@ -6,7 +6,7 @@ | |||
# IMAGE_NAME specifies a name of the candidate image used for testing. | |||
# The image has to be available before this script is executed. | |||
# | |||
declare -a COMMON_WEB_APPS=({gunicorn-config-different-port,gunicorn-different-port,django-different-port,standalone,setup,setup-requirements,django,numpy,app-home,locale,pipenv,pipenv-and-micropipenv-should-fail,app-module,pyuwsgi-pipenv{% if spec.version.startswith("3.") %},micropipenv,standalone-custom-pypi-index{% endif %}}-test-app) | |||
declare -a COMMON_WEB_APPS=({gunicorn-config-different-port,gunicorn-different-port,gunicorn-python-configfile-different-port,django-different-port,standalone,setup,setup-requirements,django,numpy,app-home,locale,pipenv,pipenv-and-micropipenv-should-fail,app-module,pyuwsgi-pipenv{% if spec.version.startswith("3.") %},micropipenv,standalone-custom-pypi-index,gunicorn-python-configfile-different-port{% endif %}}-test-app) |
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.
The first occurrence of gunicorn-python-configfile-different-port
should be removed from here.
443f6fe
to
3751b24
Compare
[test] |
No description provided.