WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
127784
WebKit Bot Watcher's Dashboard: Add support for using HTTP authentication with Buildbot
https://bugs.webkit.org/show_bug.cgi?id=127784
Summary
WebKit Bot Watcher's Dashboard: Add support for using HTTP authentication wit...
Daniel Bates
Reported
2014-01-28 09:25:52 PST
Implement support for querying a Buildbot that requires HTTP authentication.
Attachments
Patch
(8.30 KB, patch)
2014-01-28 09:34 PST
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Patch
(6.74 KB, patch)
2014-01-29 12:02 PST
,
Daniel Bates
ap
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Daniel Bates
Comment 1
2014-01-28 09:34:49 PST
Created
attachment 222461
[details]
Patch
Alexey Proskuryakov
Comment 2
2014-01-28 09:58:11 PST
Comment on
attachment 222461
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=222461&action=review
> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Utilities.js:44 > + if (this.status && this.status !== 200 && options.hasOwnProperty("callbackOnLoadError") && options.callbackOnLoadError === true) { > + callback({error: this.statusText, errorHTTPCode: this.status}); > + return; > + }
I think that we should always notify the caller of errors. This is actually needed in other code paths too, as we could implement some optimizations if we could track whether iteration data is currently loading, or the load already failed. An additional callback argument would work. Alternatively, we could pass an object with multiple optional callback functions, e.g. JSON.load(url, { onsuccess = function(data) {...}, onparseerror=..., onloaderror=... }).
Daniel Bates
Comment 3
2014-01-29 12:02:33 PST
Created
attachment 222588
[details]
Patch
Alexey Proskuryakov
Comment 4
2014-01-29 12:12:01 PST
Comment on
attachment 222588
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=222588&action=review
> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueue.js:65 > + get needsAuthentication() > + { > + return this.buildbot.needsAuthentication; > + },
I'd just put this check inline - we don't need to create a concept of a queue that needs authentication, it's the server that does.
Daniel Bates
Comment 5
2014-01-29 12:13:07 PST
(In reply to
comment #2
)
> (From update of
attachment 222461
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=222461&action=review
> > > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Utilities.js:44 > > + if (this.status && this.status !== 200 && options.hasOwnProperty("callbackOnLoadError") && options.callbackOnLoadError === true) { > > + callback({error: this.statusText, errorHTTPCode: this.status}); > > + return; > > + } > > I think that we should always notify the caller of errors. This is actually needed in other code paths too, as we could implement some optimizations if we could track whether iteration data is currently loading, or the load already failed. > > An additional callback argument would work. Alternatively, we could pass an object with multiple optional callback functions, e.g. JSON.load(url, { onsuccess = function(data) {...}, onparseerror=..., onloaderror=... }).
I chose to separate into different bugs the change to teach JSON.load() to optionally send an XMLHTTPRequest with credentials from the change to avoid excessive HTTP authentication prompts. This bug is for the former and
bug #127849
is for the latter, which includes adding support for a failure callback function to JSON.load().
Daniel Bates
Comment 6
2014-01-29 12:13:38 PST
(In reply to
comment #4
)
> (From update of
attachment 222588
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=222588&action=review
> > > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueue.js:65 > > + get needsAuthentication() > > + { > > + return this.buildbot.needsAuthentication; > > + }, > > I'd just put this check inline - we don't need to create a concept of a queue that needs authentication, it's the server that does.
Will inline before landing.
Daniel Bates
Comment 7
2014-01-29 12:18:29 PST
Committed
r163034
: <
http://trac.webkit.org/changeset/163034
>
Daniel Bates
Comment 8
2014-01-29 13:17:46 PST
Fixed syntax issue when using JSONP requests and committed the fix in <
http://trac.webkit.org/changeset/163040
>.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug