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

switch to using a thread for the live server #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danchr
Copy link

@danchr danchr commented Aug 3, 2017

Running the server in a thread works quite well, and prevents any
issues with communication between processes. Furthermore, it fixes
stdio capturing to work as expected.

Tested by using something similar in my own test suite on Python 3.6, and by running Flask-Testing's own suite on Python 2.6, 2.7 and 3.6.

Running the server in a thread works quite well, and prevents any
issues with communication between processes. Furthermore, it fixes
stdio capturing to work as expected.
@jcomo
Copy link
Collaborator

jcomo commented Aug 3, 2017

Excellent idea! Thanks for the contribution.

I am not familiar with Werkzeug's API to create servers but it looks like there is an option for threading here: https://github.com/pallets/werkzeug/blob/master/werkzeug/serving.py#L576. Is there a reason you're using a thread over the threaded mixin server?

@danchr
Copy link
Author

danchr commented Aug 11, 2017

Is there a reason you're using a thread over the threaded mixin server?

Well, yes; I wanted to use the serve_forever() method, with blocks the thread. I don't think the threaded server helps with that, but rather allows the server to process multiple requests concurrently.

I tried switching my application to the Flask configuration system, and noticed that it didn't seem to affect the live server with these changes. I'll try to come up with a test to check whether that's a regression.

@jcomo
Copy link
Collaborator

jcomo commented Aug 14, 2017

That was my thought - wanted to make sure I had the right idea. This is great. Let's get a test for that regression if you can manage it and then we'll merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants