-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fix umask option bug(issues#1622) #1632
Conversation
b269a58
to
ced208e
Compare
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.
Could you also convert the example shared in #1622 to a test case?
gunicorn/config.py
Outdated
if x.startswith('0') and not x.lower().startswith('0x'): | ||
# for compatible with octal numbers in python3 | ||
# for compatible with octal numbers in python3 | ||
if re.match('0(\d)', x, re.IGNORECASE): |
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.
I'd use r''
to avoid subtle bugs.
ced208e
to
bcfd534
Compare
@berkerpeksag thanks, I add those examples to |
I think the problem here is that our test code doesn't call |
Nevermind, the existing test was wrong. |
tests/test_config.py
Outdated
@@ -375,6 +375,8 @@ def test_reload(options, expected): | |||
|
|||
@pytest.mark.parametrize("options, expected", [ | |||
(["--umask 0", "myapp:app"], 0), |
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 should read:
(["--umask", "0", "myapp:app"], 0),
tests/test_config.py
Outdated
@@ -375,6 +375,8 @@ def test_reload(options, expected): | |||
|
|||
@pytest.mark.parametrize("options, expected", [ | |||
(["--umask 0", "myapp:app"], 0), | |||
(["--umask 0o0", "myapp:app"], 0), |
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.
Same as above:
(["--umask", "0o0", "myapp:app"], 0),
(["--umask", "0x0", "myapp:app"], 0),
bcfd534
to
d06aaab
Compare
@berkerpeksag, thanks, you are right. I fixed the tests |
Thanks! |
Fix umask option bug. issue_1622