Opened 5 years ago

Closed 5 years ago

#2818 closed enhancement (fixed)

[nine] rewrite buildsteps to not write to status

Reported by: dustin Owned by: dustin
Priority: major Milestone: 0.9.0
Version: 0.8.7p1 Keywords:
Cc:

Description

Part of #2646.

Buildsteps currently write to both status pickles and the data API. They should only write to the data API.

Change History (13)

comment:1 Changed 5 years ago by dustin

I've been stalled on this for three months :(

This bug includes:

  • making waitingForLocks a local (private) attribute of BuildStep (why was this ever on the status?!)
  • building a fake self.step_status for old-style steps with methods
    • setText
    • setText2
  • removing uses of self.step_status in master/buildbot/process/buildstep.py - in most cases these are already paired with a data API call, so they can simply be removed. Calls to setText can be replaced with calls to updateSummary.
  • removing progress/ETA support (I think there's already a bug to re-add this in a more functional fashion)
  • removing setSkipped calls (nothing ever calls isSkipped anyway)
  • adding data API support for hidden steps
  • fudge things in LoggingBuildStep so that they continue to work as much as possible, even though the step_status argument to evaluateCommand probably doesn't do what the user expects anymore
  • removing setStepStatus and related methods for setting up step status
  • update tests
    • for changes above
    • the default expected final status for every step has changed from status_text=['generic'], and will need to be updated in each test. It's probably best to make this argument optional, since it's not particularly interesting in most cases.

Much of this has been done in https://github.com/djmitche/buildbot/tree/bug2818 -- mostly missing the updated tests.

Last edited 5 years ago by dustin (previous) (diff)

comment:2 Changed 5 years ago by dustin

Updated status:

  • [DONE] making waitingForLocks a local (private) attribute of BuildStep? (why was this ever on the status?!)
  • [DONE] building a fake self.step_status for old-style steps with methods setText, setText2
  • [DONE] removing uses of self.step_status in master/buildbot/process/buildstep.py - in most cases these are already paired with a data API call, so they can simply be removed. Calls to setText can be replaced with calls to updateSummary.
  • [DONE] removing setSkipped calls (nothing ever calls isSkipped anyway)
  • [DONE] fudge things in LoggingBuildStep? so that they continue to work as much as possible, even though the step_status argument to evaluateCommand probably doesn't do what the user expects anymore
  • [DONE] removing setStepStatus and related methods for setting up step status
  • update tests
    • [DONE] for changes above
    • [WORK IN PROGRESS] the default expected final status for every step has changed from status_text=generic?, and will need to be updated in each test. It's probably best to make this argument optional, since it's not particularly interesting in most cases.
      • trying desperately to keep old-style step descriptions the same, but boy are they confusing!
      • renamed the expectOutcome status_text parameter, so un-edited tests are obvious
    • [DONE] if we call updateSummary, it can't be illegal (call realUpdateSummary instead)
  • removing progress/ETA support (I think there's already a bug to re-add this in a more functional fashion)
  • adding data API support for hidden steps
  • [NEW] remove master/buildbot/status/buildstep.py, moving it to buildbot.util.pickle
  • [NEW] convert stepStateStrings to take only a single string, since that's all we produce with the summary handling.

comment:3 Changed 5 years ago by dustin

This is almost ready for a pull req, with the rest to be done in branch 2818b. I just need to test BuildStep.getCurrentSummary and getResultSummary.

Once that's done, in the new branch:

  • remove progress/ETA support (I think there's already a bug to re-add this in a more functional fashion)
  • add data API support for hidden steps
  • remove master/buildbot/status/buildstep.py, moving it to buildbot.util.pickle
  • [WIP] convert stepStateStrings to take only a single string, since that's all we produce with the summary handling

comment:4 Changed 5 years ago by dustin

Oh, and bug2818 needs some real-world testing, too!

comment:5 Changed 5 years ago by Pierre Tardy <pierre.tardy@…>

In 8b72fb079f74ff380e2bdb50e2b42d188a318f85:

Merge branch 'bug2818' of github.com:djmitche/buildbot into pr1238

comment:6 Changed 5 years ago by dustin

#2590 covers the progress part

comment:7 Changed 5 years ago by Dustin J. Mitchell <dustin@…>

In d830cde7a802e7613d088ac766eb3ea7443999ba:

Remove Progress/ETA support.

This is part of removing the reliance on status APIs in bug 2818. The
progress interface wasn't very good, anyway, and will be re-implemented
in bug 2590.

comment:8 Changed 5 years ago by Mikhail Sobolev <mss@…>

In bb5433120fe911f578465fa0349e4cce941a81dd:

Merge pull request #1268 from djmitche/2818c

Remove Progress/ETA support.

See ticket:2818

comment:9 Changed 5 years ago by Mikhail Sobolev <mss@…>

In 32a8eba59c09aa3716ee09acf8c1d346600308d1:

Merge pull request #1269 from djmitche/bug2818b

Change state_strings to state_string

See ticket:2818

comment:10 Changed 5 years ago by Mikhail Sobolev <mss@…>

In ae0e33d970174f668dbabe38a2789e480435f10f:

Merge pull request #1271 from djmitche/bug2949

move support for description{,Done,Suffix} entirely to BuildStep?

See ticket:2818
Fixes ticket:2949

comment:11 Changed 5 years ago by dustin

  • remove progress/ETA support (I think there's already a bug to re-add this in a more functional fashion)
    commit d830cde7a802e7613d088ac766eb3ea7443999ba
    Author: Dustin J. Mitchell <dustin@buildbot.net>
    Date:   Sun Oct 19 17:14:08 2014 -0400
    
        Remove Progress/ETA support.
        
        This is part of removing the reliance on status APIs in bug 2818.  The
        progress interface wasn't very good, anyway, and will be re-implemented
        in bug 2590.
    
  • remove master/buildbot/status/buildstep.py, moving it to buildbot.util.pickle
    commit 8b72fb079f74ff380e2bdb50e2b42d188a318f85
    Merge: 97a0944 4c12a18
    Author: Pierre Tardy <pierre.tardy@intel.com>
    Date:   Thu Oct 2 15:31:48 2014 +0200
    
        Merge branch 'bug2818' of github.com:djmitche/buildbot into pr1238
    
    
  • convert stepStateStrings to take only a single string, since that's all we produce with the summary handling
    commit 32a8eba59c09aa3716ee09acf8c1d346600308d1
    Merge: d7badd3 69b22d2
    Author: Mikhail Sobolev <mss@mawhrin.net>
    Date:   Tue Oct 21 10:33:37 2014 +0300
    
        Merge pull request #1269 from djmitche/bug2818b
        
        Change state_strings to state_string
        
        See ticket:2818
    

So, still todo:

  • add data API support for hidden steps

comment:13 Changed 5 years ago by dustin

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.