This looks good. It'll certainly be easier to debug jobs this way.
One question, though.
> 18 @@ -262,7 +265,8 @@
> 19 def _logOopsId(self, oops_id):
> 20 """Report oopses by id to the log."""
> 21 if self.logger is not None:
> 22 - self.logger.info('Job resulted in OOPS: %s' % oops_id)
> 23 + self.logger.info('Job resulted in OOPS: %s' % (
> 24 + oops_id))
> 25 self.oops_ids.append(oops_id)
I'm not sure I see the need for this change; it doesn't look like the line was over the character limit, and functionally the two are identical, aren't they?
Given that I think they're identical, this doesn't block approval, I'm just curious what the intent was.
This looks good. It'll certainly be easier to debug jobs this way.
One question, though.
> 18 @@ -262,7 +265,8 @@ info('Job resulted in OOPS: %s' % oops_id) info('Job resulted in OOPS: %s' % ( ids.append( oops_id)
> 19 def _logOopsId(self, oops_id):
> 20 """Report oopses by id to the log."""
> 21 if self.logger is not None:
> 22 - self.logger.
> 23 + self.logger.
> 24 + oops_id))
> 25 self.oops_
I'm not sure I see the need for this change; it doesn't look like the line was over the character limit, and functionally the two are identical, aren't they?
Given that I think they're identical, this doesn't block approval, I'm just curious what the intent was.