Bug 39749 - webkit-patch fails to create bugs since it started assigning the owner
Summary: webkit-patch fails to create bugs since it started assigning the owner
Status: RESOLVED DUPLICATE of bug 40039
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 39975 (view as bug list)
Depends on: 40039
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-26 10:11 PDT by Justin Schuh
Modified: 2010-06-02 02:45 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.50 KB, patch)
2010-05-31 18:29 PDT, Lucas De Marchi
no flags Details | Formatted Diff | Diff
Patch (1.63 KB, patch)
2010-05-31 19:52 PDT, Lucas De Marchi
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Schuh 2010-05-26 10:11:45 PDT
I assume it's a permission error because I don't have commit or edit bug rights yet. I've pasted the whole stack, but the relevant part seems to be "ValueError: control 'assigned_to' is disabled"

Traceback (most recent call last):
  File "/Users/jschuh/dev/WebKit/WebKitTools/Scripts/webkit-patch", line 64, in <module>
    main()
  File "/Users/jschuh/dev/WebKit/WebKitTools/Scripts/webkit-patch", line 59, in main
    WebKitPatch(__file__).main()
  File "/Users/jschuh/dev/WebKit/WebKitTools/Scripts/webkitpy/tool/multicommandtool.py", line 302, in main
    result = command.check_arguments_and_execute(options, args, self)
  File "/Users/jschuh/dev/WebKit/WebKitTools/Scripts/webkitpy/tool/multicommandtool.py", line 113, in check_arguments_and_execute
    return self.execute(options, args, tool) or 0
  File "/Users/jschuh/dev/WebKit/WebKitTools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 43, in execute
    self._sequence.run_and_handle_errors(tool, options, self._prepare_state(options, args, tool))
  File "/Users/jschuh/dev/WebKit/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py", line 66, in run_and_handle_errors
    self._run(tool, options, state)
  File "/Users/jschuh/dev/WebKit/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py", line 60, in _run
    step(tool, options).run(state)
  File "/Users/jschuh/dev/WebKit/WebKitTools/Scripts/webkitpy/tool/steps/createbug.py", line 48, in run
    state["bug_id"] = self._tool.bugs.create_bug(state["bug_title"], state["bug_description"], blocked=state.get("bug_blocked"), component=self._options.component, cc=cc)
  File "/Users/jschuh/dev/WebKit/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py", line 707, in create_bug
    self.browser["assigned_to"] = assignee
  File "/Users/jschuh/dev/WebKit/WebKitTools/Scripts/webkitpy/thirdparty/autoinstalled/clientform/ClientForm.py", line 2899, in __setitem__
    raise ValueError(str(e))
ValueError: control 'assigned_to' is disabled
Comment 1 Eric Seidel (no email) 2010-05-31 10:29:43 PDT
I think we should consider rolling the original patch out unless Adam has time to fix it.
Comment 2 Lucas De Marchi 2010-05-31 18:26:47 PDT
*** Bug 39975 has been marked as a duplicate of this bug. ***
Comment 3 Lucas De Marchi 2010-05-31 18:29:15 PDT
Created attachment 57507 [details]
Patch

This patch fixes the issue, logging if it was not possible to set the assignee of the new bug
Comment 4 Darin Adler 2010-05-31 18:30:11 PDT
Comment on attachment 57507 [details]
Patch

I'm not sure it's important to log that.

Why is it OK to remove the "if assignee" condition?
Comment 5 Lucas De Marchi 2010-05-31 18:40:05 PDT
(In reply to comment #4)
> (From update of attachment 57507 [details])
> I'm not sure it's important to log that.
> 
> Why is it OK to remove the "if assignee" condition?

Because of the previous check, if assignee == None ;-)
Comment 6 Lucas De Marchi 2010-05-31 18:43:33 PDT
(In reply to comment #4)
> (From update of attachment 57507 [details])
> I'm not sure it's important to log that.

I'd rather let there to remember us non-committers that we have to commit more stuff :-)
Comment 7 Eric Seidel (no email) 2010-05-31 18:45:57 PDT
Comment on attachment 57507 [details]
Patch

The log is wrong.  It has nothing to do with being a committer.  It has to do with if you have editbugs permissions in bugzilla.  We could instead check the enabled/disbled state (or presence of) the assigned_to field.
Comment 8 Chris Jerdonek 2010-05-31 18:47:51 PDT
> +++ b/WebKitTools/ChangeLog
> +        Continue to upload a patch even if it's not possible to set the assignee
> +        due to control being disabled. This happens in case user is not a
> +        committer.

Doesn't this happen only if the user doesn't have the "editbugs" privilege?
See here, for example:

http://www.bugzilla.org/docs/3.2/en/html/userpreferences.html#permissionsettings

Also see this webkit-dev thread:

https://lists.webkit.org/pipermail/webkit-dev/2009-December/010980.html

> +++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py
> +        try:
>              self.browser["assigned_to"] = assignee
> +        except ValueError:
> +            log('User is not a committer. It is not possible to assign bug to %s' % assignee)
> +            pass

Again, I think it has more to do with the "editbugs" privilege rather than
being a committer.  (I'm not sure if we're automatically granting committers
bugs.webkit.org privileges.)  Also, will the user see this message?  If not, is
there a way to display a message to the user so the user can know to take
corrective action?  Such a message should probably include instructions on
how to ask for the "editbugs" privilege.  See the webkit-dev thread above
for details.  The message should probably also include the assignee name
that was attempted.
Comment 9 Chris Jerdonek 2010-05-31 18:51:37 PDT
(In reply to comment #8)
> for details.  The message should probably also include the assignee name
> that was attempted.

Never mind this part.  You already have this.
Comment 10 Lucas De Marchi 2010-05-31 19:52:59 PDT
Created attachment 57510 [details]
Patch

I've put a check on disabled attribute, as Eric suggested. What do you think?
Comment 11 Lucas De Marchi 2010-05-31 19:54:42 PDT
(In reply to comment #8)
> bugs.webkit.org privileges.)  Also, will the user see this message?  If not, is

Yes, it does appear.
Comment 12 Eric Seidel (no email) 2010-05-31 22:49:15 PDT
Comment on attachment 57510 [details]
Patch

I would have broken this out into a separate function at this point.

+            log('User does not have editbugs privilege. It is not possible to assign bug to %s' % assignee)

should be:

+            log('%s does not have editbugs privilege. It is not possible to assign bug to %s' % (self.username, assignee))

Should also probably direct them to how to request the privilege (I'm not sure how, for now we should ask them to email webkit-dev I guess).

Also this current patch will log about the "None" assignee which is not what you mean to do.
Comment 13 Lucas De Marchi 2010-06-01 00:27:55 PDT
(In reply to comment #12)
> Should also probably direct them to how to request the privilege (I'm not sure how, for now we should ask them to email webkit-dev I guess).
> 
Do I put anything about directing to email webkit-dev?

> Also this current patch will log about the "None" assignee which is not what you mean to do.
None will eval to false on last check, thus anything is printed. It will print anything if user set assignee to "" too.
Comment 14 Lucas De Marchi 2010-06-02 02:45:10 PDT
#40039 gave a simple solution that has been committed.

*** This bug has been marked as a duplicate of bug 40039 ***