launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/celery-worker-discard-stderr into lp:launchpad.

Commit message:
Discard stderr from test celery workers, since we don't read from it anyway.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/celery-worker-discard-stderr/+merge/364158

Perhaps ideally we might read stdout and stderr and attach it as test details.  However, in the meantime, having a pipe that we never read from is a time-bomb: if you're especially unlucky then you might end up spending most of a day poring over several million lines of strace output trying to work out why your celery workers are hanging, not that I speak from experience or anything.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/celery-worker-discard-stderr into lp:launchpad.

=== modified file 'lib/lp/services/job/tests/__init__.py'
--- lib/lp/services/job/tests/__init__.py	2019-03-06 14:52:01 +0000
+++ lib/lp/services/job/tests/__init__.py	2019-03-08 14:10:32 +0000
@@ -42,11 +42,12 @@
         '--include', 'lp.services.job.tests.celery_helpers',
     )
     # Mostly duplicated from lazr.jobrunner.tests.test_celerytask.running,
-    # but we throw away stdout.
+    # but we throw away stdout and stderr since we never read from them
+    # anyway and we don't want them to block.
     with open('/dev/null', 'w') as devnull:
         proc = subprocess.Popen(
             ('bin/celery',) + cmd_args, stdout=devnull,
-            stderr=subprocess.PIPE, cwd=cwd)
+            stderr=devnull, cwd=cwd)
         try:
             yield proc
         finally: