Add manual run and retry for status updates.

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

david
Review Board
release-3.0.x
baa7313...
reviewboard

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. Can you add:

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

    Can you add:

    ...
    
    Type:
        unicode
    
  4. Can you swap these so they'll be in alphabetical order?

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

    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. 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. Should use assertSpyCalledWith().

  8. Should use assertSpyCalledWith().

  9. Should use assertIn().

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

  10. 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)
Loading...