Add web API resources for tool executions and executables.
Review Request #6139 — Created July 23, 2014 and submitted
This adds the two new resources tool_executions and tool_executables, and also
brings back the old review_bot_review resource (left unmodified).At a high level, this API now supports multiple profiles/configurations per
tool, allows us to manually execute tools, and allows us to log and track the
status of tool executions.A tool execution contains information on a tool profile that is currently
being executed or has been executed on some diff of a review request. This
resource supports getting a specified tool execution, getting a list of tool
executions for a review request ID and diff revision (with the optional
parametersstatusandget-latest, the latter of which will be very useful
for the manual execution UI), creating a new tool execution and adding an
execution request to the message queue, and updating a tool execution's status
and result (used by the workers).The tool executables resource provides a list of tool profiles that the user is
allowed to manually execute, for some review request ID and diff revision.
Did GET, POST, PUT requests on tool-executions with all possible request
parameters, and GET requests on tool-executables.Went through multiple manual execution scenarios (using the change in /r/6192).
- Got a list of executable tool profiles.
- Removed all manual permissions on a profile, and saw that it was no longer
on the list of executables.
- Tried to create an execution, specifying a profile not on the list.
- Created an execution, specifying a profile on the list this time.
- Got the list of executions for the review request/diff, and saw the 'queued'
status for the above execution.
- Saw that the execution request was received by a worker. The worker updated
the tool execution with the 'succeeded' status and JSON review, and
published a review on the review request. The last-updated time was also
automatically updated.
- Checked the list of executable tool profiles, and saw that the just-executed
profile was no longer on the list.
- Tried to execute the just-executed profile, and failed.
- Tried passing invalid review request IDs, diff revisions, and profile IDs,
and verified that the requests all failed. - Tried passing a profile ID for which the user did not have permission to
manually execute, and verified that that the profile was not listed as an
executable and a PERMISSION_DENIED error for the POST. - Tried to update the status of an execution (not logged in as the Review Bot
user) and was denied permission.
| Description | From | Last Updated |
|---|---|---|
|
Col: 13 E123 closing bracket does not match indentation of opening bracket's line |
 reviewbot
|
|
|
Why did you choose to keep your import in the scope of this function? Will it break something else if … |
PE PeterTran | |
|
Col: 13 E123 closing bracket does not match indentation of opening bracket's line |
reviewbot
|
- Testing Done:
-
Did GET, POST, PUT requests on tool-executions with all possible request
parameters, and GET requests on tool-executables. ~ Went through multiple manual execution scenarios, after updating notify() (to
~ Went through multiple manual execution scenarios (using the change in /r/6192).
- work with profiles instead of tools) and adding code to ProcessReviewRequest - (to update the tool execution with its status after publishing the review). - Got a list of executable tool profiles. - Removed all manual permissions on a profile, and saw that it was no longer on the list of executables. - Tried to create an execution, specifying a profile not on the list. - Created an execution, specifying a profile on the list this time. - Got the list of executions for the review request/diff, and saw the 'queued' status for the above execution. ~ - Saw that the execution request was received by a worker. The worker then ~ published a review on the review request, and updated the tool execution with ~ the 'succeeded' status. The last-updated time was also automatically updated. ~ - Saw that the execution request was received by a worker. The worker updated ~ the tool execution with the 'succeeded' status and JSON review, and ~ published a review on the review request. The last-updated time was also + automatically updated. - Checked the list of executable tool profiles, and saw that the just-executed profile was no longer on the list. - Tried to execute the just-executed profile, and failed. - Tried passing invalid review request IDs, diff revisions, and profile IDs,
and verified that the requests all failed.
- Tried passing a profile ID for which the user did not have permission to
manually execute, and verified that that the profile was not listed as an
executable and a PERMISSION_DENIED error for the POST.
- Tried to update the status of an execution (not logged in as the Review Bot
user) and was denied permission.
- Tried passing invalid review request IDs, diff revisions, and profile IDs,
- Commit:
-
6e17198f593f8aca6972e46672dcd662ab490b94167d7f01362a4847a3f5083d3f97a0754d71cd13
-
-
Why did you choose to keep your import in the scope of this function? Will it break something else if it's imported for the entire file?
If you wanted this kept in the scope of the function, can it be imported within the first few lines of the function?
Sorry if there is an obvious reason behind this.
-
I've given this a sweep, but haven't had a chance to look over it really in depth yet.
Things look great though, so I don't suspect there would be any large problems once I
really dig into it. I'm comfortable with us landing this now and making fixups later
if problems are discovered.Thanks a lot, this change is great :D
Tool: Pyflakes Processed Files: extension/reviewbotext/models.py extension/reviewbotext/resources.py Tool: PEP8 Style Checker Processed Files: extension/reviewbotext/models.py extension/reviewbotext/resources.py