Skip to content
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

Pserve --reload does not keep worker arguments. Fixes #2961 #2962

Merged
merged 5 commits into from
Mar 6, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions pyramid/scripts/pserve.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@
from pyramid.path import AssetResolver
from pyramid.settings import aslist


def main(argv=sys.argv, quiet=False):
command = PServeCommand(argv, quiet=quiet)
return command.run()


class PServeCommand(object):

description = """\
Expand Down Expand Up @@ -113,7 +115,6 @@ class PServeCommand(object):
"passed here.",
)


ConfigParser = configparser.ConfigParser # testing
loadapp = staticmethod(loadapp) # testing
loadserver = staticmethod(loadserver) # testing
Expand All @@ -124,9 +125,11 @@ def __init__(self, argv, quiet=False):
self.args = self.parser.parse_args(argv[1:])
if quiet:
self.args.verbose = 0
if self.args.reload:
self.worker_kwargs = {'argv': argv, "quiet": quiet}
self.watch_files = []

def out(self, msg): # pragma: no cover
def out(self, msg): # pragma: no cover
if self.args.verbose > 0:
print(msg)

Expand Down Expand Up @@ -203,6 +206,7 @@ def open_browser():
'pyramid.scripts.pserve.main',
reload_interval=int(self.args.reload_interval),
verbose=self.args.verbose,
worker_kwargs=self.worker_kwargs
)
return 0

Expand Down Expand Up @@ -239,22 +243,24 @@ def open_browser():
msg = ''
self.out('Exiting%s (-v to see traceback)' % msg)


# For paste.deploy server instantiation (egg:pyramid#wsgiref)
def wsgiref_server_runner(wsgi_app, global_conf, **kw): # pragma: no cover
def wsgiref_server_runner(wsgi_app, global_conf, **kw): # pragma: no cover
from wsgiref.simple_server import make_server
host = kw.get('host', '0.0.0.0')
port = int(kw.get('port', 8080))
server = make_server(host, port, wsgi_app)
print('Starting HTTP server on http://%s:%s' % (host, port))
server.serve_forever()


# For paste.deploy server instantiation (egg:pyramid#cherrypy)
def cherrypy_server_runner(
app, global_conf=None, host='127.0.0.1', port=None,
ssl_pem=None, protocol_version=None, numthreads=None,
server_name=None, max=None, request_queue_size=None,
timeout=None
): # pragma: no cover
): # pragma: no cover
"""
Entry point for CherryPy's WSGI server

Expand Down Expand Up @@ -361,5 +367,6 @@ def cherrypy_server_runner(

return server

if __name__ == '__main__': # pragma: no cover

if __name__ == '__main__': # pragma: no cover
sys.exit(main() or 0)
20 changes: 19 additions & 1 deletion pyramid/tests/test_scripts/test_pserve.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import mock
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't currently use mock in the the Pyramid tests, instead we create dummy objects that have just the functionality required.

I don't think it is a great idea to add yet another dependency...

import os
import unittest
from pyramid.tests.test_scripts import dummy

here = os.path.abspath(os.path.dirname(__file__))


class TestPServeCommand(unittest.TestCase):
def setUp(self):
from pyramid.compat import NativeIO
Expand Down Expand Up @@ -48,10 +50,11 @@ def test_parse_vars_good(self):
inst.loadserver = self._get_server

app = dummy.DummyApp()

def get_app(*args, **kwargs):
app.global_conf = kwargs.get('global_conf', None)
inst.loadapp = get_app

inst.loadapp = get_app
inst.run()
self.assertEqual(app.global_conf, {'a': '1', 'b': '2'})

Expand All @@ -77,6 +80,21 @@ def test_config_file_finds_watch_files(self):
os.path.abspath(os.path.join(here, '*.py')),
])

def test_reload_call_hupper_with_correct_args(self):
with mock.patch('pyramid.scripts.pserve.hupper') as hupper_mock:
hupper_mock.is_active.side_effect = [False, True]
inst = self._makeOne('--reload', 'development.ini')
inst.loadserver = mock.MagicMock()
inst.loadapp = mock.MagicMock()
inst.run()
hupper_mock.start_reloader.assert_called_with(
'pyramid.scripts.pserve.main',
reload_interval=1,
verbose=1,
worker_kwargs={'argv': ['pserve', '--reload', 'development.ini'],
'quiet': False})


class Test_main(unittest.TestCase):
def _callFUT(self, argv):
from pyramid.scripts.pserve import main
Expand Down
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,10 @@
]

testing_extras = tests_require + [
'mock',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a -1 on this, we use dummy objects everywhere else.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way this dependencies are only installed when you run tests which is not something you do when you use the project.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I understand that.

'nose',
'coverage',
'virtualenv', # for scaffolding tests
'virtualenv', # for scaffolding tests
]

setup(name='pyramid',
Expand Down