Closed Bug 963956 Opened 8 years ago Closed 8 years ago
Prints could be tidier
59.10 KB, image/png
881 bytes, patch
|Details | Diff | Splinter Review|
881 bytes, patch
|Details | Diff | Splinter Review|
I like that blobber reports its uploads to tbpl via TinderboxPrints, but it is hard to copy/paste the huge URLs, and the messages seem overly verbose. See attached screenshot. Instead of: (blobuploader) - INFO - TinderboxPrint: Uploaded logcat-emulator-5556.log to http: //mozilla-releng-blobs.s3.amazonaws.com/blobs/mozilla-central/sha512/0969b88fe80b58c89de4b41b4338070043395a6006a1231b4ff836533fc845d12faaab7e109c17753361094c4b410e79aa4a87baa925e2d6a15eca62605fd3ef I would prefer something like: (blobuploader): logcat-emulator-5556.log where the file name, "logcat-emulator-5556.log" is a link to the aws url.
It's pretty straight-forward to tune the output for the Tinderboxprint. We would change the message here: https://github.com/MihaiTabara/blobuploader/blob/master/blobberc.py#L148 The problem is I don't know what exactly am I to print. How do we create a hyperlink there? Feels wrong to generate HTML knowing that's going to be rendered in the page. Is there somehow a way to define rules for content display in TinderboxPrint?
Once you learn about why that space after colon exists and what you have to do to avoid it, you'll look back on that feeling about generating HTML as the last time you thought the world was right and good and proper and reasonable ;) The right thing to do is for someone to fix bug 845388, but... not likely. The thing you have to hack around instead is in https://hg.mozilla.org/webtools/tbpl/file/67c667cc7446/js/MachineResult.js#l84 and keep in mind that's greedy. Lines with no colon or <br/> are left alone, but for any line with a colon followed by a character, or a <br/> followed by a character, tbpl will split the line there and insert a space. The useless history lesson behind that is that tbpl had to reverse-engineer TinderboxPrints which had been designed to both avoid unwanted wrapping in the giant Tinderbox <table>, and to avoid unwanted widening of columns in that <table> once we started running so many jobs that it was multiple horizontal scrolls wide even on a huge monitor. So there were/are TinderboxPrints like "<a href="">Foo:1</a>" and like "<a href="">Foo<br/>1</a>", but alas for the people who now want to have things like "Foo http://etc" there weren't any which had a URL, either raw or in a link, which contained either the only colon or the last colon in the TinderboxPrint, so tbpl didn't reverse-engineer the existence of that. If you want to follow the example of the last person who had to swallow his bile and hack around that regex, his choice was "TinderboxPrint: upload <a title='hazards_results' href='http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-linux64-br-haz/20140126090425'>results</a>: complete"
Summary: blobber TinderboxPrint's could be tidier → blobber TinderboxPrints could be tidier
Using github is a significant barrier to entry for me, but if someone wanted to fix this months-long grotesquery, log.info("TinderboxPrint: <a href='%s'>%s</a>: uploaded", blob_url, filename) would make blobber usable.
@philor: I am sorry for this disappointing delay. I was supposed to handle it. Kinda' lost track of this bug in my triage. Added the aforementioned line here - https://github.com/MihaiTabara/blobuploader/blob/master/blobberc.py#L148 Released 1.1.2 version of blobuploader - the tar.gz is here: https://pypi.python.org/simple/blobuploader/ As soon as Rail agrees with this, I'll patch the mozharness version of blobber.
Uploaded to the local mirror: http://pypi.pub.build.mozilla.org/pub/blobuploader-1.1.2.tar.gz
Comment on attachment 8411002 [details] [diff] [review] Bump blobuploader version to 1.1.2 https://hg.mozilla.org/build/mozharness/rev/9fccc23740f6 Until buildduty merges it to production it will be available on Cedar only.
mozharness patch live in production: http://hg.mozilla.org/build/mozharness/rev/d5a7c46032fe :)
Crap, I'm sorry, I should have looked at what you landed. The most significant character in my suggestion was the colon in "</a>:" because that's what throws tbpl's regex off the track, and makes it screw with that rather than screwing with the colon in the URL. Without that colon, now we're creating links to http: // with a space.
Crap, that's my bad.Should've copy-paste-ed that one. Fixed it - https://github.com/MihaiTabara/blobuploader/blob/master/blobberc.py#L148 Released 1.1.3 version of blobber - tar.gz is here - https://pypi.python.org/simple/blobuploader/
* after blobuploader gets puppetized in mozilla's mirrors
Comment on attachment 8411398 [details] [diff] [review] Bump blobuploader version to 1.1.3 The new tarball uploaded Pushed to default: https://hg.mozilla.org/build/mozharness/rev/bd6fa61ff840
And https://tbpl.mozilla.org/?tree=Cedar&rev=094e808eb41f&jobname=Ubuntu%20VM%2012.04%20cedar%20opt%20test%20web-platform-tests looks and works just fine :)
Actually, it does looking good :-) Nice!
mozharness patch is in production: http://hg.mozilla.org/build/mozharness/rev/9accabdd4358 :)
Can we close this?
I couldn't ask for more -- looks great now!
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.