Add web API resources for tool executions and executables.

Review Request #6139 — Created July 23, 2014 and submitted

Information

ReviewBot
master
167d7f0...

Reviewers

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
parameters status and get-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

reviewbotreviewbot

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

reviewbotreviewbot
reviewbot
  1. 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
    
    
  2. extension/reviewbotext/resources.py (Diff revision 1)
     
     
    Col: 13
     E123 closing bracket does not match indentation of opening bracket's line
    
  3. 
      
anselina
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        extension/reviewbotext/models.py
        extension/reviewbotext/resources.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        extension/reviewbotext/models.py
        extension/reviewbotext/resources.py
    
    
  2. extension/reviewbotext/resources.py (Diff revision 2)
     
     
    Col: 13
     E123 closing bracket does not match indentation of opening bracket's line
    
  3. 
      
PE
  1. 
      
  2. extension/reviewbotext/resources.py (Diff revision 2)
     
     

    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.

    1. Importing it at the top would actually throw an ImportError when you try to load the extension. When Review Board loads the extensions, reviewbotext.extension:ReviewBotExtension acts as the entry point. If you look at extension.py then, you'll see from reviewbotext.resources import (…). This means that importing ReviewBotExtension at the top of resources.py would cause a circular dependency. We can avoid this by just postponing the import since we know ReviewBotExtension must have already been instantiated by then. :)

  3. 
      
SM
  1. 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

  2. 
      
anselina
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (a2b151f)
Loading...