Closed
Bug 664402
Opened 13 years ago
Closed 9 years ago
retry.py should print process output incrementally
Categories
(Release Engineering :: General, defect, P4)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: bhearsum, Unassigned)
References
Details
(Whiteboard: [kanban:engops:https://mozilla.kanbanize.com/ctrl_board/6/1932] [automation][retry])
Attachments
(1 file, 2 obsolete files)
6.38 KB,
patch
|
Details | Diff | Splinter Review |
retry.py stores up all the output of a process until it completes, then prints it. This is a bit of pain, especially since we're using it a lot in Buildbot now. We don't get to see incremental output, and because there's no process output, there's a greater chance of steps that run retry.py getting timed out.
Updated•13 years ago
|
Assignee: nobody → rail
Priority: P3 → P2
Comment 2•13 years ago
|
||
Not sure how seek/read/tell may work here since subprocess may write to the file at the same time...
Another solution would be dropping output search feature and use stdout/stderr directly. ATM we don't use output search but bufferization hits us sometime.
Attachment #557101 -
Flags: feedback?(catlee)
Comment 3•13 years ago
|
||
Comment on attachment 557101 [details] [diff] [review]
retry.py: print as it comes
Review of attachment 557101 [details] [diff] [review]:
-----------------------------------------------------------------
::: buildfarm/utils/retry.py
@@ +53,5 @@
> + last_stderr = stderr.tell()
> + if console_stdout:
> + print console_stdout,
> + if console_stderr:
> + print >> sys.stderr, console_stderr,
I'm not sure if this will work. I think that the subprocess inherits the file descriptors for stdout/stderr, which means that they also share the same offset. I'm worried that by seeking backwards, you'll affect where future output of the process is written.
If you could test whether this is true or not, that would be great!
Attachment #557101 -
Flags: feedback?(catlee) → feedback-
Comment 4•13 years ago
|
||
Attachment #557436 -
Flags: review?(catlee)
Updated•13 years ago
|
Attachment #557101 -
Attachment is obsolete: true
Comment 5•13 years ago
|
||
Comment on attachment 557436 [details] [diff] [review]
use NamedTemporaryFile and separate file objects
Review of attachment 557436 [details] [diff] [review]:
-----------------------------------------------------------------
::: buildfarm/utils/retry.py
@@ +6,5 @@
> Return code is 0 if cmd succeeds, and is 1 if cmd fails after all the retries.
> """
>
> import time, subprocess, logging, os, sys, re
> +from tempfile import NamedTemporaryFile
This allows to have a real file accessible by file()
@@ +11,3 @@
> from operator import xor
> log = logging.getLogger()
> +logging.basicConfig(format='%(asctime)-15s %(message)s', level=logging.DEBUG)
Make logging prettier.
@@ +58,5 @@
> + if console_stderr:
> + print >> sys.stderr, console_stderr,
> + except IOError:
> + pass
> +
Using a separate file descriptor (opened with 'r' mode by default) allows us to use independent seek points (I tested this by printing tell()). I also ignore IOErrors here, mostly EOF.
Updated•13 years ago
|
Attachment #557436 -
Flags: review?(catlee)
Comment 6•13 years ago
|
||
Looks like the only way to read stdout/sterr non blocking is to use Queue.get_nowait() + threading... Otherwise (fctnl) it doesn't work on windows.
If this approach is OK, I'd like to write some tests for retry.py.
Attachment #557436 -
Attachment is obsolete: true
Attachment #558770 -
Flags: feedback?(catlee)
Updated•13 years ago
|
Attachment #558770 -
Flags: feedback?(catlee)
Comment 7•13 years ago
|
||
I'll need to try another approach and use the following scenario: read stdout/stderr of the command, print it to stdout/stderr and a temporary file (if needed) for processing.
Updated•13 years ago
|
Priority: P2 → P4
Updated•13 years ago
|
Component: Release Engineering → Release Engineering: Automation
OS: Linux → All
Priority: P4 → --
QA Contact: release → catlee
Hardware: x86_64 → All
Whiteboard: [automation][retry]
Updated•13 years ago
|
Priority: -- → P4
Assignee | ||
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
Updated•10 years ago
|
Whiteboard: [automation][retry] → [kanban:engops:https://mozilla.kanbanize.com/ctrl_board/6/1932] [automation][retry]
Reporter | ||
Comment 9•9 years ago
|
||
Redo doesn't have this problem! WE should probably kill retry.py and use redo's command instead.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
Assignee | ||
Updated•6 years ago
|
Component: General Automation → General
You need to log in
before you can comment on or make changes to this bug.
Description
•