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

Fix --restart #91

Merged
merged 1 commit into from Aug 20, 2018
Merged

Conversation

luk3thomas
Copy link
Contributor

I've always had trouble with the --restart flag. I want it to restart web servers on file changes, but I always seem to get an address in use error no matter what I do.

Steps to reproduce

$ touch a
$ filewatcher -I --restart a 'python -m SimpleHTTPServer'
Serving HTTP on 0.0.0.0 port 8000 ...

Then in another window, run touch a to make filewatcher restart the server. I get something like this.

Traceback (most recent call last):
  File "/Users/lthomas1/.asdf/installs/python/2.7/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/Users/lthomas1/.asdf/installs/python/2.7/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/Users/lthomas1/.asdf/installs/python/2.7/lib/python2.7/SimpleHTTPServer.py", line 218, in <module>
    test()
  File "/Users/lthomas1/.asdf/installs/python/2.7/lib/python2.7/SimpleHTTPServer.py", line 214, in test
    BaseHTTPServer.test(HandlerClass, ServerClass)
  File "/Users/lthomas1/.asdf/installs/python/2.7/lib/python2.7/BaseHTTPServer.py", line 592, in test
    httpd = ServerClass(server_address, HandlerClass)
  File "/Users/lthomas1/.asdf/installs/python/2.7/lib/python2.7/SocketServer.py", line 408, in __init__
    self.server_bind()
  File "/Users/lthomas1/.asdf/installs/python/2.7/lib/python2.7/BaseHTTPServer.py", line 108, in server_bind
    SocketServer.TCPServer.server_bind(self)
  File "/Users/lthomas1/.asdf/installs/python/2.7/lib/python2.7/SocketServer.py", line 419, in server_bind
    self.socket.bind(self.server_address)
  File "/Users/lthomas1/.asdf/installs/python/2.7/lib/python2.7/socket.py", line 222, in meth
    return getattr(self._sock,name)(*args)
socket.error: [Errno 48] Address already in use

The new process we're starting isn't able to listen to the port because the old process is still hanging around.

I started digging around and I noticed the variable child_pid is always nil, ever after filewatcher cycles the process. I think we need to add an explicit return in the ensure block to alter the return value of the restart command. I did some googling and this blog post seems to confirm that suspicion http://blog.leshill.org/blog/2009/11/17/ensure-with-explicit-return.html.

@luk3thomas luk3thomas changed the title Add explicit return Fix --restart Aug 18, 2018
@thomasfl thomasfl merged commit b0300b2 into filewatcher:master Aug 20, 2018
@thomasfl
Copy link
Collaborator

@luk3thomas Thank's for taking time to dig into to the problem and submitting a pull request. The solution was very simple.

I am happy to see that the --restart option is being used.

@luk3thomas
Copy link
Contributor Author

My pleasure!

What is the process for creating a new release?

@AlexWayfer
Copy link
Member

What is the process for creating a new release?

I can do this. But I'd like to see some tests for this (that don't work without this change and work with it).

@AlexWayfer
Copy link
Member

@thomasfl, can you please install Cirrus CI for this repo? I don't have access. I want to try another CI because we have some phantom fails. Also, this CI has all platforms (Linux, macOS and Windows) in one place.

Or we can move this repo to some organization, like filewatcher/filewatcher. There is also #90.

@AlexWayfer
Copy link
Member

What is the process for creating a new release?

Done today as 1.1.0.

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.

None yet

3 participants