[Catalyst-dev] Re: Re: PATCH: refactor Catalyst::Engine::HTTP::Restarter to standalone module

[Mark Stosberg]

Matt S Trout wrote:

> On Sun, Jul 22, 2007 at 04:30:48PM -0400, Mark Stosberg wrote:
>> Matt S Trout wrote:
>> 
>> > On Sun, Jul 22, 2007 at 08:58:42AM -0400, Mark Stosberg wrote:
>> >> Hello,
>> >> 
>> >> I like the auto-restart feature that is in the Catalyst
>> >> HTTP::Restarter server. I wanted to use the restarting trick with a
>> >> project based on HTTP::Server::Simple, so I looked to see how tied
>> >> this functionality was to Catalyst. Here's what I found:
>> >> 
>> >> Catalyst::Engine::HTTP::Restarter::Watcher is not functionally tied to
>> >> Catalyst at all. It just shares a namespace.
>> >> 
>> >> Catalyst::Engine::HTTP::Restarter depends on no functionality from
>> >> Catalyst but was still coupled with it through a base-class
>> >> relationship which could be easily avoided.
>> >> 
>> >> The attached patch moves these to modules into the
>> >> HTTP::Server::Restarter namespace, and makes slight alternations to
>> >> the other Catalyst files so that the functionality remains the same,
>> >> as well as the dependencies.
>> > 
>> > Wouldn't Devel::Restart::Automatic or similar be more sensible
>> > name-wise?
>> > 
>> > I don't see that there needs to be anything HTTP-dependent in the code
>> > you're factoring out.
>> 
>> The approach is HTTP specific. The monitor acts as HTTP client and sends
>> a special "RESTART" requeest over HTTP when a change is detected. I
>> looked at two different approaches today that involved sending a SIGHUP
>> signal instead, and I couldn't get either to work. I suspect the authors
>> settled on this approach because it was reliable by comparison.
> 
> I suspect SIGUSR1 would work just fine; the 'authors' (mostly Sebastian)
> don't actually understand signal handling, I had to do quite a lot of
> cleanup work to make the child-related stuff sane.

I continued to look at this issue tonight, and came up with a patch for
HTTP::Server::Simple that uses simple signal handling. Unless there is a
reason to allow restarting of the web server from remote machines (!!),
switching to this kind of signal handling system does seem like a good
solution. 

http://rt.cpan.org/Ticket/Display.html?id=28423#txn-347392

>> So, I think HTTP::Server::Restarter is good that part. The second
>> module, "Watcher.pm" is not HTTP-specific, it just monitors a directory
>> for
>> changes, based some options.  It is similar to: "File::Monitor".
>> 
>> http://search.cpan.org/~andya/File-Monitor-v0.0.5/
>> 
>> I suggest it be named "File::Monitor::Simple"
> 
> Is there any reason not to switch to File::Monitor while we're here?

Yes, File::Monitor is much larger than you need. The current Catalyst
solution does leverage some existing files: 

 use File::Find;
 use File::Modified;
 use File::Spec;

Catalyst has a nice small, simple alternative to File::Monitor. Maybe it's
File::Monitor::Tiny. :)

> Please try and subscribe again, and contact me at the provided address if
> you continue to have problems.

I was able to subscribe this time around, and this has been moved to
the -dev list now. 

   Mark