Closed
Bug 1191808
Opened 7 years ago
Closed 7 years ago
Change all the custom WebAPIError codes to be greater than 1000
Categories
(MozReview Graveyard :: General, defect, P2)
MozReview Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mdoglio, Assigned: mdoglio)
References
Details
Attachments
(3 files)
From https://bugzilla.mozilla.org/show_bug.cgi?id=1109218#c144: General comment about all of the WebAPIError's introduced in the commits - All error codes should be greater than 1000 (which RB core guarantees it will not use itself). Also, I'm thinking it might just be best to stick them all in the same file (including ones introduced by unrelated commits here). That way finding a particular error to use, or incrementing to the next available id is easier.
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Comment 1•7 years ago
|
||
Mozreview: WebAPIError codes greater than 1000 (Bug 1191808); r?smacleod
Attachment #8651802 -
Flags: review?(smacleod)
Assignee | ||
Comment 2•7 years ago
|
||
Mozreview: Add new WebAPIError AUTOLAND_CONFIGURATION_ERROR; r?smacleod
Attachment #8651803 -
Flags: review?(smacleod)
Assignee | ||
Comment 3•7 years ago
|
||
Mozreview: substitute BAD_UPDATE_CREDENTIALS with PERMISSION_DENIED; r?smacleod
Attachment #8651806 -
Flags: review?(smacleod)
Comment 4•7 years ago
|
||
Comment on attachment 8651802 [details] MozReview Request: Mozreview: WebAPIError codes greater than 1000 (Bug 1191808); r?smacleod https://reviewboard.mozilla.org/r/16987/#review15235 ::: pylib/mozreview/mozreview/errors.py:13 (Diff revision 1) > Can you update the commit message to include the reason for this change please.
Attachment #8651802 -
Flags: review?(smacleod) → review+
Comment 5•7 years ago
|
||
https://reviewboard.mozilla.org/r/16987/#review15237 ::: pylib/mozreview/mozreview/autoland/resources.py:32 (Diff revision 1) > -from mozreview.decorators import webapi_scm_groups_required > + NOT_PUSHED_PARENT_REVIEW_REQUEST, The trailing paren `)` should be added in this commit not the next, so this commit isn't broken.
Comment 6•7 years ago
|
||
Comment on attachment 8651803 [details] MozReview Request: Mozreview: Add new WebAPIError AUTOLAND_CONFIGURATION_ERROR; r?smacleod https://reviewboard.mozilla.org/r/16989/#review15239 ::: pylib/mozreview/mozreview/autoland/resources.py:31 (Diff revision 1) > - NOT_PUSHED_PARENT_REVIEW_REQUEST, > + NOT_PUSHED_PARENT_REVIEW_REQUEST) Make sure to move this change to the first commit. ::: pylib/mozreview/mozreview/errors.py:52 (Diff revision 1) > BAD_UPDATE_CREDENTIALS = WebAPIError( > 1008, > "Bad or missing AutolandRequest update credentials.", > http_status=401) # 401 Unauthorized I don't believe 1008 is checked or used by anyone yet (and 1007 wasn't either). How about we just slip in a change to make this 1007 so there aren't any gaps :)
Attachment #8651803 -
Flags: review?(smacleod)
Comment 7•7 years ago
|
||
Comment on attachment 8651803 [details] MozReview Request: Mozreview: Add new WebAPIError AUTOLAND_CONFIGURATION_ERROR; r?smacleod https://reviewboard.mozilla.org/r/16989/#review15241 Meant to put a ship-it on the last review.
Attachment #8651803 -
Flags: review+
Comment 8•7 years ago
|
||
Comment on attachment 8651806 [details] MozReview Request: Mozreview: substitute BAD_UPDATE_CREDENTIALS with PERMISSION_DENIED; r?smacleod https://reviewboard.mozilla.org/r/16991/#review15243 ::: pylib/mozreview/mozreview/autoland/resources.py:302 (Diff revision 1) > - return BAD_UPDATE_CREDENTIALS > + return PERMISSION_DENIED Even though we're using the built in codes from review board for these things, you can still provide a custom message if you think it's useful. See the method on `WebAPIError` and the example in its docstring here: https://github.com/djblets/djblets/blob/6c8dd31b0bd49e0d0ecad4cce1718b923fc4e7dd/djblets/webapi/errors.py#L52-L58
Attachment #8651806 -
Flags: review?(smacleod) → review+
Comment 9•7 years ago
|
||
https://reviewboard.mozilla.org/r/16989/#review15239 > I don't believe 1008 is checked or used by anyone yet (and 1007 wasn't either). How about we just slip in a change to make this 1007 so there aren't any gaps :) Ah, I see in the next commit you remove this. NVM, carry on.
Assignee | ||
Comment 11•7 years ago
|
||
Comment on attachment 8651802 [details] MozReview Request: Mozreview: WebAPIError codes greater than 1000 (Bug 1191808); r?smacleod Mozreview: WebAPIError codes greater than 1000 (Bug 1191808); r?smacleod Error codes should be greater than 1000 (which RB core guarantees it will not use itself). Let's stick them all in the same file (including ones introduced by unrelated commits here). That way finding a particular error to use, or incrementing to the next available id is easier.
Attachment #8651802 -
Flags: review+ → review?(smacleod)
Assignee | ||
Comment 12•7 years ago
|
||
Comment on attachment 8651803 [details]
MozReview Request: Mozreview: Add new WebAPIError AUTOLAND_CONFIGURATION_ERROR; r?smacleod
Mozreview: Add new WebAPIError AUTOLAND_CONFIGURATION_ERROR; r?smacleod
Attachment #8651803 -
Flags: review+ → review?(smacleod)
Assignee | ||
Comment 13•7 years ago
|
||
Comment on attachment 8651806 [details]
MozReview Request: Mozreview: substitute BAD_UPDATE_CREDENTIALS with PERMISSION_DENIED; r?smacleod
Mozreview: substitute BAD_UPDATE_CREDENTIALS with PERMISSION_DENIED; r?smacleod
Attachment #8651806 -
Flags: review+ → review?(smacleod)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → mdoglio
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•7 years ago
|
Attachment #8651802 -
Flags: review?(smacleod) → review+
Assignee | ||
Updated•7 years ago
|
Attachment #8651803 -
Flags: review?(smacleod) → review+
Assignee | ||
Updated•7 years ago
|
Attachment #8651806 -
Flags: review?(smacleod) → review+
Updated•6 years ago
|
Product: Developer Services → MozReview
You need to log in
before you can comment on or make changes to this bug.
Description
•