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

[Matt S Trout]

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.
 
> 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?

> > Once we've settled one a name, I'd be happy to set up a directory under
> > Catalyst svn for the factored out code and give you commit to it, then we
> > can move the core across to it once everybody's had a chance to play with
> > the factored-out code and stabilise it.
> 
> Great.
> 
> > And can we move this to the -dev list where it belongs, please? 
> 
> I did try that first. Sending the message directly there was rejected
> automatically, and attempting to subscribe seemed to be me in
> account-moderation-limbo. I'd be happy to be post there once I'm able to.

It uses the same mailman install as this list does, configured in a standard
fashion, and I see no mail from you so evidently you didn't try and contact
the listadmin.

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

-- 
      Matt S Trout       Need help with your Catalyst or DBIx::Class project?
   Technical Director    Want a managed development or deployment platform?
 Shadowcat Systems Ltd.  Contact mst (at) shadowcatsystems.co.uk for a quote
http://chainsawblues.vox.com/             http://www.shadowcatsystems.co.uk/