Bug 40389 – make rietveld upload faster and avoid posting to bug on errors

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ojan Vafai]

make rietveld upload faster and avoid posting to bug on errors

[Ojan Vafai]

Created attachment 58297 [details]
Patch

[Adam Barth]

Comment on attachment 58297 [details]
Patch

Looks good, but R- because of the race condition.

WebKitTools/Scripts/webkitpy/common/net/bugzilla.py:275
 +              return self._fetch_bug(bugs[0]).in_rietveld_queue_patches()[0]
Is there a race condition where the first query sees the bug as having a rietveld? patch but this request doesn't?  Wouldn't that cause use to throw an exception here?

WebKitTools/Scripts/webkitpy/tool/commands/queues.py:338
 +          self._reject_patch(patch.id(), message)
I'd pass the patch here for consistency with other queues.

[Ojan Vafai]

Created attachment 58301 [details]
Patch

[Ojan Vafai]

(In reply to comment #2)
> WebKitTools/Scripts/webkitpy/common/net/bugzilla.py:275
>  +              return self._fetch_bug(bugs[0]).in_rietveld_queue_patches()[0]
> Is there a race condition where the first query sees the bug as having a rietveld? patch but this request doesn't?  Wouldn't that cause use to throw an exception here?

Whoops. Fixed.

> WebKitTools/Scripts/webkitpy/tool/commands/queues.py:338
>  +          self._reject_patch(patch.id(), message)
> I'd pass the patch here for consistency with other queues.

Actually, it seems like all the other queues pass the patch id around in the functions called from handle_script_error and handle_unexpected_error.

[Adam Barth]

Comment on attachment 58301 [details]
Patch

I think you have a typo, but looks good otherwise.

WebKitTools/Scripts/webkitpy/common/net/bugzilla.py:274
 +          def fetch_patch(url):
Normally we'd make this a private method.

WebKitTools/Scripts/webkitpy/common/net/bugzilla.py:275
 +              bugs = self._fetch_bug_ids_advanced_query(rietveld_queue_url)
rietveld_queue_url -> url?

WebKitTools/Scripts/webkitpy/common/net/bugzilla.py:276
 +              patches = self._fetch_bug(bugs[0]).in_rietveld_queue_patches()
Does in_rietveld_queue_patches cover r? too?

[Ojan Vafai]

(In reply to comment #5)
> WebKitTools/Scripts/webkitpy/common/net/bugzilla.py:274
>  +          def fetch_patch(url):
> Normally we'd make this a private method.

Done.

> WebKitTools/Scripts/webkitpy/common/net/bugzilla.py:275
>  +              bugs = self._fetch_bug_ids_advanced_query(rietveld_queue_url)
> rietveld_queue_url -> url?
> 
> WebKitTools/Scripts/webkitpy/common/net/bugzilla.py:276
>  +              patches = self._fetch_bug(bugs[0]).in_rietveld_queue_patches()
> Does in_rietveld_queue_patches cover r? too?

Doh. Sorry, I didn't get around to running the queue through all the in-rietveld? patches and didn't hit this. Anyways, fixed both issues and confirmed that r? patches get uploaded correctly.

[Ojan Vafai]

Committed r60924: <http://trac.webkit.org/changeset/60924>