LGTM
On Fri, Jun 11, 2010 at 7:33 AM, John Admanski <[email protected]> wrote: > Restores proper support for handling unexpected client terminations > after the server_job.record refactoring. > > The original record implementation just dumped whatever status logs > the client sent it into the status.log, so if the client died on you > there was never a problem with the client leaving the server-side > logging in a funky state. Now that client job records are being > properly routed though the server-side job.record a client death can > leave the server logging in a broken state. > > This implementation adds a special get_record_context method which can > be used by the autotest.py code to retrieve the current job.record > context at the start of the client job, and then restore it at the end > so that the server side logging is put back into a known-good state. > > It also removes a bunch of self._record_prefix uses in the server_job > code which were rendered useless by the job.record changes, but > weren't removed. > > Signed-off-by: John Admanski <[email protected]> > > --- autotest/server/autotest.py 2010-06-09 14:46:54.000000000 -0700 > +++ autotest/server/autotest.py 2010-06-10 09:52:02.000000000 -0700 > @@ -735,6 +735,7 @@ > local_results = collector.server_results_dir > self.host.job.add_client_log(hostname, remote_results, > local_results) > + job_record_context = self.host.job.get_record_context() > > section = 0 > start_time = time.time() > @@ -783,6 +784,7 @@ > self.host.job.postprocess_client_state(state_path) > self.host.job.remove_client_log(hostname, remote_results, > local_results) > + job_record_context.restore() > > # should only get here if we timed out > assert timeout > @@ -842,9 +844,6 @@ > """Partial file object to write to both stdout and > the status log file. We only implement those methods > utils.run() actually calls. > - > - Note that this class is fairly closely coupled with server_job, as it > - uses special job._ methods to actually carry out the loggging. > """ > status_parser = re.compile(r"^AUTOTEST_STATUS:([^:]*):(.*)$") > test_complete_parser = re.compile(r"^AUTOTEST_TEST_COMPLETE:(.*)$") > --- autotest/server/server_job.py 2010-06-09 14:46:54.000000000 -0700 > +++ autotest/server/server_job.py 2010-06-10 09:52:02.000000000 -0700 > @@ -67,6 +67,17 @@ > self._indent -= 1 > > > + def get_context(self): > + """Returns a context object for use by job.get_record_context.""" > + class context(object): > + def __init__(self, indenter, indent): > + self._indenter = indenter > + self._indent = indent > + def restore(self): > + self._indenter._indent = self._indent > + return context(self, self._indent) > + > + > class server_job_record_hook(object): > """The job.record hook for server job. Used to inject WARN messages from > the console or vlm whenever new logs are written, and to echo any logs > @@ -171,7 +182,6 @@ > self.args = args > self.machines = machines > self._client = client > - self._record_prefix = '' > self.warning_loggers = set() > self.warning_manager = warning_manager() > self._ssh_user = ssh_user > @@ -217,8 +227,9 @@ > self.harness = None > > # set up the status logger > + self._indenter = status_indenter() > self._logger = base_job.status_logger( > - self, status_indenter(), 'status.log', 'status.log', > + self, self._indenter, 'status.log', 'status.log', > record_hook=server_job_record_hook(self)) > > > @@ -602,14 +613,9 @@ > Underlying method for running something inside of a group. > """ > result, exc_info = None, None > - old_record_prefix = self._record_prefix > try: > self.record('START', subdir, name) > - self._record_prefix += '\t' > - try: > - result = function(*args, **dargs) > - finally: > - self._record_prefix = old_record_prefix > + result = function(*args, **dargs) > except error.TestBaseException, e: > self.record("END %s" % e.exit_status, subdir, name) > exc_info = sys.exc_info() > @@ -653,20 +659,15 @@ > get_kernel_func: a function that returns a string > representing the kernel version. > """ > - > - old_record_prefix = self._record_prefix > try: > self.record('START', None, 'reboot') > - self._record_prefix += '\t' > reboot_func() > except Exception, e: > - self._record_prefix = old_record_prefix > err_msg = str(e) + '\n' + traceback.format_exc() > self.record('END FAIL', None, 'reboot', err_msg) > raise > else: > kernel = get_kernel_func() > - self._record_prefix = old_record_prefix > self.record('END GOOD', None, 'reboot', > optional_fields={"kernel": kernel}) > > @@ -749,6 +750,20 @@ > return subdirectory > > > + def get_record_context(self): > + """Returns an object representing the current job.record context. > + > + The object returned is an opaque object with a 0-arg restore method > + which can be called to restore the job.record context (i.e. > indentation) > + to the current level. The intention is that it should be used when > + something external which generate job.record calls (e.g. an autotest > + client) can fail catastrophically and the server job record state > + needs to be reset to its original "known good" state. > + > + �...@return: A context object with a 0-arg restore() method.""" > + return self._indenter.get_context() > + > + > def record_summary(self, status_code, test_name, reason='', > attributes=None, > distinguishing_attributes=(), child_test_ids=None): > """Record a summary test result. > _______________________________________________ > Autotest mailing list > [email protected] > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest > _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
