Skip to content

Let retry.py flush stdout after each log message#19754

Merged
foolip merged 1 commit intomasterfrom
foolip/retry-flush
May 7, 2020
Merged

Let retry.py flush stdout after each log message#19754
foolip merged 1 commit intomasterfrom
foolip/retry-flush

Conversation

@foolip
Copy link
Member

@foolip foolip commented Oct 17, 2019

This is to avoid the subprocess output appearing before the
"Running %s [try %d/%d]" message.

@foolip
Copy link
Member Author

foolip commented Oct 17, 2019

https://github.com/web-platform-tests/wpt/pull/19750/checks?check_run_id=263749188 shows the order being wrong before this change.

@foolip
Copy link
Member Author

foolip commented Oct 17, 2019

In https://github.com/web-platform-tests/wpt/pull/19754/checks?check_run_id=263770797 the order is correct.

@foolip foolip force-pushed the foolip/retry-flush branch from 1bf622e to 8ab5b37 Compare October 17, 2019 10:47
@foolip foolip marked this pull request as ready for review October 17, 2019 11:53
@foolip foolip assigned jgraham and unassigned jugglinmike Oct 17, 2019
@foolip
Copy link
Member Author

foolip commented Oct 17, 2019

@jgraham should I update the docker image too?

@jgraham
Copy link
Contributor

jgraham commented Oct 17, 2019

Yes.

@foolip
Copy link
Member Author

foolip commented Oct 17, 2019

@jgraham done!

@foolip foolip force-pushed the foolip/retry-flush branch from 9a45e85 to 3cb58e9 Compare October 18, 2019 11:27
@foolip
Copy link
Member Author

foolip commented Oct 18, 2019

I've triggered full Chrome and Firefox runs to check that the new docker image doesn't break everything:
https://tools.taskcluster.net/groups/CN4eZJbdR5OraIFOrgOyfA
https://tools.taskcluster.net/groups/HFMq9rRUQKqppMNDWVIPsA

@foolip foolip mentioned this pull request Oct 22, 2019
Merged
@foolip
Copy link
Member Author

foolip commented Oct 25, 2019

Ping @jgraham

This was referenced Oct 25, 2019
Closed
Closed
jgraham
jgraham requested changes Dec 18, 2019
View reviewed changes
@gsnedders gsnedders closed this Jan 24, 2020
@gsnedders gsnedders deleted the foolip/retry-flush branch January 24, 2020 18:06
@gsnedders gsnedders restored the foolip/retry-flush branch January 24, 2020 18:50
@Hexcles Hexcles reopened this Jan 24, 2020
@foolip
Let retry.py flush stdout after each log message
0e74076 
This is to avoid the subprocess output appearing before the
"Running %s [try %d/%d]" message.
@foolip foolip force-pushed the foolip/retry-flush branch from 3cb58e9 to 0e74076 Compare May 6, 2020 11:01
@wpt-pr-bot wpt-pr-bot added the ci label May 6, 2020
@foolip
Copy link
Member Author

foolip commented May 6, 2020

@stephenmcgruer @Hexcles I'd like to get this landed and I see you updated the image in #23190. Can either of you help generate and upload the new image that would make this PR work?

https://github.com/web-platform-tests/wpt/runs/637380261#step:4:1001 shows how this is still a problem, makes it hard to see what happened in which try.

@stephenmcgruer
Copy link
Contributor

stephenmcgruer commented May 6, 2020

Can either of you help generate and upload the new image that would make this PR work?

Absolutely :). I have checked out this PR, built webplatformtests/wpt:0.37 and pushed it to docker hub. Close/open to kick TC.

@stephenmcgruer stephenmcgruer reopened this May 6, 2020
@foolip
Copy link
Member Author

foolip commented May 6, 2020

I've filed #23441 about the failing Azure Pipelines jobs.

@jgraham are you happy to see this merge now?

@foolip
Copy link
Member Author

foolip commented May 6, 2020

Also, ahem, thanks @stephenmcgruer for updating the image for me!!

@foolip foolip merged commit ca16f38 into master May 7, 2020
@foolip foolip deleted the foolip/retry-flush branch May 7, 2020 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stephenmcgruer stephenmcgruer stephenmcgruer approved these changes

@jgraham jgraham jgraham approved these changes

@gsnedders gsnedders Awaiting requested review from gsnedders

@jugglinmike jugglinmike Awaiting requested review from jugglinmike

@Hexcles Hexcles Awaiting requested review from Hexcles

@KyleJu KyleJu Awaiting requested review from KyleJu

@LukeZielinski LukeZielinski Awaiting requested review from LukeZielinski

Assignees

@jgraham jgraham

Labels

ci docker infra Taskcluster

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@foolip @jgraham @stephenmcgruer @gsnedders @jugglinmike @Hexcles @wpt-pr-bot @moz-wptsync-bot