New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Output stdout and stderr when nailgun fails to connect #5892

Merged
merged 1 commit into from Jun 1, 2018

Conversation

Projects
None yet
3 participants
@illicitonion
Copy link
Contributor

illicitonion commented Jun 1, 2018

Fixes #5853

@illicitonion illicitonion requested a review from baroquebobcat Jun 1, 2018

@kwlzn

kwlzn approved these changes Jun 1, 2018

Copy link
Member

kwlzn left a comment

lgtm

@stuhood

stuhood approved these changes Jun 1, 2018

Copy link
Member

stuhood left a comment

Would be good to get a Kris shipit, but looks good. Thanks!

@@ -183,6 +183,7 @@ def _await_socket(self, timeout):
"""Blocks for the nailgun subprocess to bind and emit a listening port in the nailgun stdout."""

This comment has been minimized.

@stuhood

stuhood Jun 1, 2018

Member

Hm. @kwlzn : I thought that we had added a custom nailgun protocol message for pids? It's interesting that this is arriving on stdout (...or perhaps this is "all socket output" instead?)

This comment has been minimized.

@kwlzn

kwlzn Jun 1, 2018

Member

this is the actual stdout stream from the nailgun java server subprocess invoke, so isn't going over the nailgun protocol afaik.

This comment has been minimized.

@stuhood

stuhood Jun 1, 2018

Member

ooh. K.

'Failed to read nailgun output after {sec} seconds!\n'
'Stdout:\n{stdout}\nStderr:\n{stderr}'.format(
sec=timeout,
stdout=accumulated_stdout,

This comment has been minimized.

@stuhood

stuhood Jun 1, 2018

Member

Since you're adding newlines, it might be worthwhile to trim this.

@stuhood stuhood requested review from kwlzn and removed request for baroquebobcat Jun 1, 2018

@stuhood stuhood merged commit ec0a077 into pantsbuild:master Jun 1, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stuhood stuhood deleted the twitter:dwagnerhall/nailgun/stdout branch Jun 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment