-
Notifications
You must be signed in to change notification settings - Fork 422
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 pserve argument ordering. #1095
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.
r+ with a changelog entry ;)
kinto/__main__.py
Outdated
pserve_argv.append('http_port={}'.format(parsed_args['port'])) | ||
pserve.main(pserve_argv) | ||
print(pserve_argv) |
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.
left over or useful info?
kinto/__main__.py
Outdated
if parsed_args['reload']: | ||
pserve_argv.append('--reload') | ||
pserve_argv.append('-v') |
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.
should we add this only if our verbose option is set?
The fact that the argument order is important seems like a bug; should we report it upstream? |
We have the exact same issue with kinto and subcommands. The command help explicits it so it doesn't seems like a bug to me. |
tests/test_main.py
Outdated
res = main(['--ini', TEMP_KINTO_INI, 'start', '-v']) | ||
assert res == 0 | ||
assert mocked_pserve.call_count == 1 | ||
assert mocked_pserve.call_args_list[0][1]['argv'][1] == '-v' |
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.
We could probably use call_args instead of call_args_list.
Refs: Pylons/pyramid#2962
Fixes #1092