From: Tyler Riddle <t.riddle@shadowcat.co.uk>
Date: Wed, 3 Oct 2012 16:58:00 +0000 (-0700)
Subject: per gatherer logs can be configured; per host concurrency can be configured; remove... 
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=7012cbfe8c285c47a696c6c8facc57f27f5ae862;p=scpubgit%2FSystem-Introspector.git

per gatherer logs can be configured; per host concurrency can be configured; remove old bug fix cruft
---

diff --git a/lib/System/Introspector/Config.pm b/lib/System/Introspector/Config.pm
index b54b7b0..f5a0199 100644
--- a/lib/System/Introspector/Config.pm
+++ b/lib/System/Introspector/Config.pm
@@ -7,9 +7,6 @@ has config => (is => 'lazy');
 
 has config_file => (is => 'ro', required => 1);
 
-#TODO pull these out
-#has log_level => (is => 'lazy');
-
 sub _build_config {
     my ($self) = @_;
     my $reader = Config::General->new($self->config_file);
@@ -18,16 +15,16 @@ sub _build_config {
     return \%config;
 }
 
-#sub _build_log_level {
-#    return $_[0]->config->{log_level}; 
-#}
-
 sub sudo_user { $_[0]->config->{sudo_user} }
 
 sub groups { sort keys %{ $_[0]->config->{group} || {} } }
 
 sub has_group { exists $_[0]->config->{group}{ $_[1] } }
 
+sub host_logs { $_[0]->config->{host_logs} }
+
+sub host_concurrency { $_[0]->config->{host_concurrency} }
+
 my $_load_host_file = sub {
     my ($self, $path) = @_;
     my $full_path = join '/', dirname($self->config_file), $path;
diff --git a/lib/System/Introspector/Gatherer.pm b/lib/System/Introspector/Gatherer.pm
index b99cfb8..a67db62 100644
--- a/lib/System/Introspector/Gatherer.pm
+++ b/lib/System/Introspector/Gatherer.pm
@@ -10,6 +10,7 @@ use System::Introspector::Logger qw( :log :dlog );
 
 has introspectors => (is => 'ro', required => 1);
 has stderr_fh => ( is => 'ro' ); 
+has watchdog_timeout => ( is => 'ro', default => sub { 300 } );
 
 sub gather_all {
     my ($self) = @_;
@@ -63,8 +64,7 @@ sub new_from_spec {
     my ($user, $host, $sudo_user) = @arg{qw( user host sudo_user )};
     my $sudo = defined($sudo_user) ? sprintf('%s@', $sudo_user) : undef;
     my $args = { introspectors => $arg{introspectors} };
-    #TODO move watchdog_timeout into an attribute for this class
-    my %connection_args = ( stderr => $arg{stderr_fh}, watchdog_timeout => 300 ); 
+    my %connection_args = ( stderr => $arg{stderr_fh} ); 
     if (defined $host) {
         my $remote = join '@', grep defined, $user, $host;
         my $conn = Object::Remote::Connection->conn_from_spec($remote, %connection_args);
@@ -78,7 +78,6 @@ sub new_from_spec {
     }
     else {
         if (defined $sudo_user) {
-            #TODO find a better way to achieve this result
             my $conn = Object::Remote::Connection->conn_from_spec($sudo_user, %connection_args);
             
             return $class->_new_direct($conn->maybe::start::connect, $args);
diff --git a/lib/System/Introspector/Logger.pm b/lib/System/Introspector/Logger.pm
index f4b3ff2..3fdea9a 100644
--- a/lib/System/Introspector/Logger.pm
+++ b/lib/System/Introspector/Logger.pm
@@ -8,36 +8,6 @@ use Object::Remote::LogDestination;
 
 use base qw(Object::Remote::Logging);
 
-#sub arg_router { 
-#    return $_[1] if defined $_[1];
-#    our $Router_Instance ||= Object::Remote::LogRouter->new(description => __PACKAGE__);
-#}
-
-#should always pass in log level and execution context;
-#specifying a logger is used when logging during
-#testing so a null output logging class can be used
-#TODO waiting to see if this really is never coming back
-#sub init_logging {
-#    my ($self, $level, $context, $output) = @_;
-#    
-#    $level = 'WARN' unless defined $level;
-#    #TODO: better way to specify log level? 
-#    $ENV{SYSTEM_INTROSPECTOR_LOG_UPTO} = $level;     
-#    
-#    unless(defined($output)) {
-#        $output = System::Introspector::Logger::Output->new({
-#            env_prefix => 'SYSTEM_INTROSPECTOR_LOG',
-#        });
-#        
-#        $output->set_execution_context($context);
-#    }
-#    
-#    my $dest = Log::Contextual::Destination->new(logger => $output);
-#    $dest->connect($self->get_router);
-#    
-#    return 1; 
-#}
-
 
 1; 
 
diff --git a/lib/System/Introspector/State.pm b/lib/System/Introspector/State.pm
index 27ec8e7..f2f5f7f 100644
--- a/lib/System/Introspector/State.pm
+++ b/lib/System/Introspector/State.pm
@@ -10,30 +10,24 @@ use System::Introspector::Logger qw( :log :dlog );
 use System::Introspector::Report;
 
 has config => (is => 'ro', required => 1);
-
 has root => (is => 'ro', required => 1);
+#undef is unlimited concurrency
 
 sub user { $_[0]->config->user }
-
 sub sudo_user { $_[0]->config->sudo_user }
 
-#TODO waiting to see if this really is never coming back
-#sub BUILD {
-#    my ($self) = @_; 
-#    System::Introspector::Logger->init_logging('controller');
-#}
-
 sub gather {
     my ($self, @groups) = @_;
     log_debug { "Starting to gather results" };
+    my $concurrent_max = $self->config->host_concurrency;
+ 
     for my $group (@groups) {
         my @hosts = $self->config->hosts;
         
         log_info { my $c = scalar(@hosts); "This gather run is for $c hosts" };
         
         while(scalar(@hosts) > 0) {
-            #TODO turn the splice size into an attribute on this class
-            my @scan = splice(@hosts, 0, 50);     
+            my @scan = splice(@hosts, 0, $concurrent_max);     
             my @waiting;
         
             Dlog_trace { my $c = scalar(@scan); "Scanning $c hosts in this gather loop" } @scan;
@@ -58,12 +52,6 @@ sub gather {
                 
                 log_debug { "Waiting on futures for host '$host'" };
                 
-                #TODO another way to solve the huge JSON problem is to 
-                #invoke the probes in from controller directly via proxy
-                #objects and receive the results from each probe as
-                #they complete - it would cause less RAM consumption for the
-                #system as a whole but requires modifying the future based
-                #syncronization logic
                 eval { ($report) = await_all @futures }; 
                 
                 if ($@) {
@@ -177,17 +165,16 @@ sub _cleanup {
 
 sub _create_gatherer {
     my ($self, %arg) = @_;
-    my $stderr_fh; 
-    
-    #TODO move into attribute for this class
-if(1) {
-    my $log_file = "log/$arg{host}.log";
-    
-    log_debug { "Logging stderr for host '$arg{host}' to '$log_file'" };
+    my $logs = $self->config->host_logs; 
+    my $stderr_fh;
     
-    open($stderr_fh, ">>", $log_file) or die "Could not open '$log_file' for write: $!";
-    
-}
+    if(defined($logs)) {
+        my $log_file = "$logs/$arg{host}.log";
+        
+        log_debug { "Logging STDERR for host '$arg{host}' to '$log_file'" };
+        
+        open($stderr_fh, ">>", $log_file) or die "Could not open '$log_file' for write: $!";
+    }
     
     my $gatherer = System::Introspector::Gatherer->new_from_spec(
         user          => $self->user,