Add manual run and retry for status updates.

Review Request #11129 — Created Aug. 10, 2020 and submitted

Information

Review Board
release-3.0.x
baa7313...

Reviewers

One common request for the status updates feature has been to allow
manual or deferred running of tools. This is desirable for a few
different situations, such as infrequently-run or expensive checkers,
test builds, etc. This change adds the framework for that.

The StatusUpdate model has a new state, NOT_YET_RUN, which will show
a button that will trigger the action. This requires that the caller (an
extension) listen to the new status_update_request_run signal to
actually run the tool.

Because it is very similar in its implementation, this additionally adds
an ability for the creator of the status update to indicate that it can
be retried. The Retry action will be shown if the status update is
marked as retryable (via extra_data), and the result of the status
update is in either error or timeout states. Like the "Run" action,
this requires handling the new signal to actually run the tool when the
user clicks the button.

The StatusUpdate API can now accept request-run in the state field,
which will trigger the new signal. This allows the frontend (or
third-party callers) to trigger the run/retry action.

Based on work by Alex Klemenchuk at /r/10311/

  • Ran python and JS unit tests.
  • Used in conjunction with a Review Bot change to use these features.
Description From Last Updated

Col: 15 'localSitePrefix' is defined but never used.

reviewbotreviewbot

Col: 49 Missing semicolon.

reviewbotreviewbot

E703 statement ends with a semicolon

reviewbotreviewbot

E703 statement ends with a semicolon

reviewbotreviewbot

Can you add: ... Type: bool

chipx86chipx86

Can you add: ... Type: unicode

chipx86chipx86

"ID"

chipx86chipx86

Can you swap these so they'll be in alphabetical order?

chipx86chipx86

Instead of checking if this is in kwargs and then fetching this in two spots, how about pulling it out …

chipx86chipx86

Hmm, we should localize this, though I noticed we don't elsewhere. In standard forms, they're typically localized and have trailing …

chipx86chipx86

Should use assertSpyCalledWith().

chipx86chipx86

Should use assertSpyCalledWith().

chipx86chipx86

Should use assertIn(). Though, can we check the explicit value, instead of just the presence?

chipx86chipx86

Same as above.

chipx86chipx86

E703 statement ends with a semicolon

reviewbotreviewbot

E703 statement ends with a semicolon

reviewbotreviewbot
Checks run (2 failed)
flake8 failed.
JSHint failed.

flake8

JSHint

david
chipx86
  1. 
      
  2. Show all issues

    Can you add:

    ...
    
    Type:
        bool
    
  3. reviewboard/reviews/models/status_update.py (Diff revision 2)
     
     
     
    Show all issues

    Can you add:

    ...
    
    Type:
        unicode
    
  4. Show all issues

    Can you swap these so they'll be in alphabetical order?

  5. reviewboard/webapi/resources/status_update.py (Diff revision 2)
     
     
     
     
     
     
    Show all issues

    Instead of checking if this is in kwargs and then fetching this in two spots, how about pulling it out first and then checking if it's None before operating on it?

  6. Show all issues

    Hmm, we should localize this, though I noticed we don't elsewhere. In standard forms, they're typically localized and have trailing periods.

    Dunno if it's worth doing that in this change, since it's not an isolated case here.

    1. None of the field errors in the API are localized.

  7. Show all issues

    Should use assertSpyCalledWith().

  8. Show all issues

    Should use assertSpyCalledWith().

  9. Show all issues

    Should use assertIn().

    Though, can we check the explicit value, instead of just the presence?

  10. Show all issues

    Same as above.

  11. 
      
david
Review request changed
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (bf6dcee)