61315 2011-05-23 15:59:34 -0700 [nrwt] don't use NetworkTranslation when uploading results 2011-05-24 23:53:18 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 tony tony abarth commit-queue dpranke ojan oldest_to_newest 408424 0 tony 2011-05-23 15:59:34 -0700 [nrwt] don't use NetworkTranslation when uploading results 408426 1 94509 tony 2011-05-23 16:01:42 -0700 Created attachment 94509 Patch 408429 2 tony 2011-05-23 16:04:11 -0700 There are a few other possible fixes: a) Switch test_results_uploader.py to using mechanize (seems like it's not worth the effort) b) Switch NetworkTransaction to catch urllib errors. Maybe this would mean making a subclass that catches urllib and mechanize errors so we can uses a mechanize free version. c) Move mechanize based error catching into the caller since it's a little strange that NetworkTransaction expects the caller to use mechanize. 408451 3 94509 ojan 2011-05-23 16:22:44 -0700 Comment on attachment 94509 Patch Seems fine. 408500 4 dpranke 2011-05-23 17:26:15 -0700 So, there's a minor change here in that if the uploader gets a 500 from app engine, it will no longer retry the upload, right? I probably would've gone for some combination of (b) or (c) since it seems like NetworkTransaction shouldn't depend on mechanize but otherwise provides a useful service, but this change is probably good enough as well. Maybe we should file a separate bug to remove the dependency on NetworkTransaction at some point (or add a FIXME)? 408604 5 94509 commit-queue 2011-05-23 20:41:17 -0700 Comment on attachment 94509 Patch Clearing flags on attachment: 94509 Committed r87124: <http://trac.webkit.org/changeset/87124> 408605 6 commit-queue 2011-05-23 20:41:22 -0700 All reviewed patches have been landed. Closing bug. 408989 7 tony 2011-05-24 10:52:25 -0700 (In reply to comment #4) > So, there's a minor change here in that if the uploader gets a 500 from app engine, it will no longer retry the upload, right? How does the retry happen? AFAICT, NetworkTransaction only catches mechanize.HTTPError and a 500 error from app engine here would raise a urllib2.HTTPError. 409495 8 dpranke 2011-05-24 23:53:18 -0700 (In reply to comment #7) > (In reply to comment #4) > > So, there's a minor change here in that if the uploader gets a 500 from app engine, it will no longer retry the upload, right? > > How does the retry happen? AFAICT, NetworkTransaction only catches mechanize.HTTPError and a 500 error from app engine here would raise a urllib2.HTTPError. Okay, lame. Turns out mechanize.HTTPError *is* (==) urllib2.HTTPError. It's just a renamed symbol (not even a subclass). If you raise mechanize.HTTPError, catch urllib2.HTTPError will catch it. We should just change NetworkTransaction to catch urllib2.HTTPError. Note that the code is kind of broken anyway since the file isn't importing mechanize at all (unless that's somehow needed for auto_install magic?) 94509 2011-05-23 16:01:42 -0700 2011-05-23 20:41:17 -0700 Patch bug-61315-20110523160048.patch text/plain 2007 tony U3VidmVyc2lvbiBSZXZpc2lvbjogODcwODkKZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBi L1Rvb2xzL0NoYW5nZUxvZwppbmRleCBlMDk4N2JmY2I5NTFjNmQwYmYyMWJmZmJiZDI3M2FjM2Rk M2M5NjA1Li5mODc4NjAzNWFkZWFjOTczZmQ4MWRmZmZlYWQ5NzgxYzIxYWFkOGE2IDEwMDY0NAot LS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcg QEAKKzIwMTEtMDUtMjMgIFRvbnkgQ2hhbmcgIDx0b255QGNocm9taXVtLm9yZz4KKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbbnJ3dF0gZG9uJ3QgdXNl IE5ldHdvcmtUcmFuc2xhdGlvbiB3aGVuIHVwbG9hZGluZyByZXN1bHRzCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MTMxNQorCisgICAgICAgIFRoaXMg YXZvaWRzIGF1dG9pbnN0YWxsaW5nIG1lY2hhbml6ZS4gIEl0IGRpZG4ndCBtYXR0ZXIgYW55d2F5 CisgICAgICAgIGJlY2F1c2UgdGVzdF9yZXN1bHRzX3VwbG9hZGVyLnB5IHdhc24ndCB1c2luZyBt ZWNoYW5pemUgdG8gbWFrZSB0aGUKKyAgICAgICAgcmVxdWVzdCBzbyBOZXR3b3JrVHJhbnNhY3Rp b246OnJ1biB3YXNuJ3QgZG9pbmcgYW55dGhpbmcgZXh0cmEKKyAgICAgICAgKGl0IG9ubHkgbWF0 dGVycyBpZiBpdCBjYXRjaGVzIG1lY2hhbml6ZS5IVFRQRXJyb3IpLgorCisgICAgICAgICogU2Ny aXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvbGF5b3V0X3BhY2thZ2UvdGVzdF9yZXN1bHRzX3Vw bG9hZGVyLnB5OgorCiAyMDExLTA1LTIzICBTaGVyaWZmIEJvdCAgPHdlYmtpdC5yZXZpZXcuYm90 QGdtYWlsLmNvbT4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByODcwNzguCmRp ZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9sYXlvdXRfcGFj a2FnZS90ZXN0X3Jlc3VsdHNfdXBsb2FkZXIucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xh eW91dF90ZXN0cy9sYXlvdXRfcGFja2FnZS90ZXN0X3Jlc3VsdHNfdXBsb2FkZXIucHkKaW5kZXgg MDMzYzhjNjNlYTQxN2JkMTQxMDg2YzBmOTM1NjIzZTRiMmE4ZWU5My4uYzg4MDlkMWQ5ZTk5NWVi NDk3YmJlZGNmMThlNDlhY2VkYWI2ZTFlNyAxMDA2NDQKLS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJr aXRweS9sYXlvdXRfdGVzdHMvbGF5b3V0X3BhY2thZ2UvdGVzdF9yZXN1bHRzX3VwbG9hZGVyLnB5 CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL2xheW91dF9wYWNrYWdl L3Rlc3RfcmVzdWx0c191cGxvYWRlci5weQpAQCAtMzQsOCArMzQsNiBAQCBpbXBvcnQgbWltZXR5 cGVzCiBpbXBvcnQgc29ja2V0CiBpbXBvcnQgdXJsbGliMgogCi1mcm9tIHdlYmtpdHB5LmNvbW1v bi5uZXQubmV0d29ya3RyYW5zYWN0aW9uIGltcG9ydCBOZXR3b3JrVHJhbnNhY3Rpb24KLQogZGVm IGdldF9taW1lX3R5cGUoZmlsZW5hbWUpOgogICAgIHJldHVybiBtaW1ldHlwZXMuZ3Vlc3NfdHlw ZShmaWxlbmFtZSlbMF0gb3IgJ2FwcGxpY2F0aW9uL29jdGV0LXN0cmVhbScKIApAQCAtMTAxLDcg Kzk5LDYgQEAgY2xhc3MgVGVzdFJlc3VsdHNVcGxvYWRlcjoKICAgICAgICAgb3JpZ190aW1lb3V0 ID0gc29ja2V0LmdldGRlZmF1bHR0aW1lb3V0KCkKICAgICAgICAgdHJ5OgogICAgICAgICAgICAg c29ja2V0LnNldGRlZmF1bHR0aW1lb3V0KHRpbWVvdXRfc2Vjb25kcykKLSAgICAgICAgICAgIE5l dHdvcmtUcmFuc2FjdGlvbih0aW1lb3V0X3NlY29uZHM9dGltZW91dF9zZWNvbmRzKS5ydW4oCi0g ICAgICAgICAgICAgICAgbGFtYmRhOiBzZWxmLl91cGxvYWRfZmlsZXMocGFyYW1zLCBmaWxlX29i anMpKQorICAgICAgICAgICAgc2VsZi5fdXBsb2FkX2ZpbGVzKHBhcmFtcywgZmlsZV9vYmpzKQog ICAgICAgICBmaW5hbGx5OgogICAgICAgICAgICAgc29ja2V0LnNldGRlZmF1bHR0aW1lb3V0KG9y aWdfdGltZW91dCkK