Adding Progress Reporting

README.md

@@ -29,6 +29,10 @@ How it works, and what that means for the numbers:
   its requests to stay within GitHub's rate limits, waits and retries when GitHub asks it to, and refuses up front if
   the remaining API budget looks too small for the repositories requested. That check is an estimate, since the
   number of jobs per commit is not known until they are fetched.
+- Large collections say what they are doing on stderr: listing many repositories, with a running count; then the
+  number of commits, API calls and rough time the fetch will take, with a running count of commits fetched. On a
+  terminal the count is redrawn in place; elsewhere a line is printed every tenth. The report itself goes to stdout,
+  so `--output=tsv` stays clean.
 
 I wrote a version of this extension before the Golang support was available for `gh`, which is still available [here](https://github.com/geoffreywiseman/gh-actuse).
 

client/client.go

@@ -4,16 +4,18 @@ package client
 import (
 	"errors"
 	"fmt"
+	"io"
 	"net/http"
 
 	"github.com/cli/go-gh/v2/pkg/api"
 	"github.com/cli/go-gh/v2/pkg/repository"
 )
 
 // New creates a new Client instance, initialized with a GH RESTClient that paces requests to stay within
-// GitHub's rate limits and retries requests that are rejected as rate limited.
-func New() Client {
-	rest, err := api.NewRESTClient(api.ClientOptions{Transport: newThrottle(http.DefaultTransport)})
+// GitHub's rate limits and retries requests that are rejected as rate limited. A notice is written to notices
+// before each wait for a rate limit, so that the user can see why the command has paused.
+func New(notices io.Writer) Client {
+	rest, err := api.NewRESTClient(api.ClientOptions{Transport: newThrottle(http.DefaultTransport, notices)})
 	if err != nil {
 		panic(err)
 	}
@@ -62,33 +64,30 @@ func (e UnexpectedHostError) Error() string {
 	return "Unexpected host: " + string(e)
 }
 
-// GetWorkflows returns a slice of Workflow instances, one for each workflow in the repository
+// GetWorkflows returns a slice of Workflow instances, one for each workflow in the repository. Pages are the
+// largest GitHub allows, and a short page ends the listing without a further call.
 func (c *Client) GetWorkflows(repository Repository) ([]Workflow, error) {
-	var page uint8 = 1
 	var workflows = make([]Workflow, 0)
-
-	for {
+	for page := 1; ; page++ {
 		wfp, err := c.getWorkflowPage(repository, page)
 		if err != nil {
 			return nil, err
 		}
-		if len(wfp) == 0 {
-			break
-		}
 		workflows = append(workflows, wfp...)
-		page++
+		if len(wfp) < perPage {
+			return workflows, nil
+		}
 	}
-	return workflows, nil
 }
 
 type workflowPage struct {
 	Workflows  []Workflow
 	TotalCount uint64 `json:"total_count"`
 }
 
-func (c *Client) getWorkflowPage(repository Repository, page uint8) ([]Workflow, error) {
+func (c *Client) getWorkflowPage(repository Repository, page int) ([]Workflow, error) {
 	response := workflowPage{}
-	url := fmt.Sprintf("repos/%s/actions/workflows?page=%d", repository.FullName, page)
+	url := fmt.Sprintf("repos/%s/actions/workflows?per_page=%d&page=%d", repository.FullName, perPage, page)
 	err := c.Rest.Get(url, &response)
 	if err != nil {
 		return nil, fmt.Errorf("could not get workflow page: %w", err)
@@ -157,12 +156,11 @@ func (c *Client) GetUser(name string) (*User, error) {
 	return &response, nil
 }
 
-// GetAllRepositories returns a list of repositories for the specified user
+// GetAllRepositories returns a list of repositories for the specified user. Pages are the largest GitHub
+// allows, and a short page ends the listing without a further call.
 func (c *Client) GetAllRepositories(user *User) ([]*Repository, error) {
-	var page uint8 = 1
 	var repos = make([]*Repository, 0)
-
-	for {
+	for page := 1; ; page++ {
 		path, err := c.getAllRepositoriesPath(user, page)
 		if err != nil {
 			return nil, err
@@ -171,21 +169,19 @@ func (c *Client) GetAllRepositories(user *User) ([]*Repository, error) {
 		if err != nil {
 			return nil, err
 		}
-		if len(rp) == 0 {
-			break
-		}
 		repos = append(repos, rp...)
-		page++
+		if len(rp) < perPage {
+			return repos, nil
+		}
 	}
-	return repos, nil
 }
 
-func (c *Client) getAllRepositoriesPath(user *User, page uint8) (string, error) {
+func (c *Client) getAllRepositoriesPath(user *User, page int) (string, error) {
 	switch user.Type {
 	case "Organization":
-		return fmt.Sprintf("orgs/%s/repos?page=%d", user.Login, page), nil
+		return fmt.Sprintf("orgs/%s/repos?per_page=%d&page=%d", user.Login, perPage, page), nil
 	case "User":
-		return fmt.Sprintf("users/%s/repos?page=%d", user.Login, page), nil
+		return fmt.Sprintf("users/%s/repos?per_page=%d&page=%d", user.Login, perPage, page), nil
 	default:
 		return "", UnexpectedUserTypeError(user.Type)
 	}

client/client_test.go

@@ -1,6 +1,7 @@
 package client
 
 import (
+	"fmt"
 	"net/url"
 	"testing"
 
@@ -82,19 +83,13 @@ func TestClient_GetWorkflows(t *testing.T) {
 	// Given
 	rest, client := getTestClient()
 	repo := Repository{ID: 1, Name: "gh-actions-usage", FullName: testRepoFullName}
-	rest.On("Get", "repos/"+testRepoFullName+"/actions/workflows?page=1", mock.Anything).
+	rest.On("Get", "repos/"+testRepoFullName+"/actions/workflows?per_page=100&page=1", mock.Anything).
 		Return(nil).
 		Run(func(args mock.Arguments) {
 			wp := args.Get(1).(*workflowPage)
 			wp.Workflows = append(wp.Workflows, Workflow{ID: 1, Name: "Build", Path: ".github/workflows/build.yml", State: "active"})
 			wp.TotalCount = 1
 		})
-	rest.On("Get", "repos/"+testRepoFullName+"/actions/workflows?page=2", mock.Anything).
-		Return(nil).
-		Run(func(args mock.Arguments) {
-			wp := args.Get(1).(*workflowPage)
-			wp.TotalCount = 0
-		})
 
 	// When
 	repos, err := client.GetWorkflows(repo)
@@ -149,18 +144,72 @@ func TestClient_GetUser_NotFound(t *testing.T) {
 	assert.Nil(t, repo)
 }
 
+func TestClient_GetWorkflows_FetchesTheNextPageAfterAFullOne(t *testing.T) {
+	// Given: a full first page and a short second one
+	rest, client := getTestClient()
+	repo := Repository{ID: 1, Name: "gh-actions-usage", FullName: testRepoFullName}
+	rest.On("Get", "repos/"+testRepoFullName+"/actions/workflows?per_page=100&page=1", mock.Anything).
+		Return(nil).
+		Run(func(args mock.Arguments) {
+			wp := args.Get(1).(*workflowPage)
+			for i := range perPage {
+				wp.Workflows = append(wp.Workflows, Workflow{ID: uint(i), Name: fmt.Sprintf("Workflow %d", i)})
+			}
+		})
+	rest.On("Get", "repos/"+testRepoFullName+"/actions/workflows?per_page=100&page=2", mock.Anything).
+		Return(nil).
+		Run(func(args mock.Arguments) {
+			wp := args.Get(1).(*workflowPage)
+			wp.Workflows = append(wp.Workflows, Workflow{ID: perPage, Name: "Workflow 100"})
+		})
+
+	// When
+	workflows, err := client.GetWorkflows(repo)
+
+	// Then: both pages are returned, and no third page is requested
+	require.NoError(t, err)
+	assert.Len(t, workflows, perPage+1)
+	rest.AssertNumberOfCalls(t, "Get", 2)
+}
+
+func TestClient_GetAllRepositories_FetchesTheNextPageAfterAFullOne(t *testing.T) {
+	// Given: a full first page and a short second one
+	rest, client := getTestClient()
+	rest.On("Get", "orgs/codiform/repos?per_page=100&page=1", mock.Anything).
+		Return(nil).
+		Run(func(args mock.Arguments) {
+			ars := args.Get(1).(*[]*Repository)
+			for i := range perPage {
+				*ars = append(*ars, &Repository{ID: uint(i), Name: fmt.Sprintf("repo-%d", i)})
+			}
+		})
+	rest.On("Get", "orgs/codiform/repos?per_page=100&page=2", mock.Anything).
+		Return(nil).
+		Run(func(args mock.Arguments) {
+			ars := args.Get(1).(*[]*Repository)
+			*ars = append(*ars, &Repository{ID: perPage, Name: "repo-100"})
+		})
+	owner := &User{ID: 1, Login: "codiform", Type: "Organization"}
+
+	// When
+	repos, err := client.GetAllRepositories(owner)
+
+	// Then: both pages are returned, and no third page is requested
+	require.NoError(t, err)
+	assert.Len(t, repos, perPage+1)
+	rest.AssertNumberOfCalls(t, "Get", 2)
+}
+
 // Success Case
 func TestClient_GetAllRepositories(t *testing.T) {
 	// Given
 	rest, client := getTestClient()
-	rest.On("Get", "users/geoffreywiseman/repos?page=1", mock.Anything).
+	rest.On("Get", "users/geoffreywiseman/repos?per_page=100&page=1", mock.Anything).
 		Return(nil).
 		Run(func(args mock.Arguments) {
 			ars := args.Get(1).(*[]*Repository)
 			*ars = append(*ars, &Repository{ID: 427462569, Name: "gh-actuse", FullName: "geoffreywiseman/gh-actuse"})
 		})
-	rest.On("Get", "users/geoffreywiseman/repos?page=2", mock.Anything).
-		Return(nil) //.
 	owner := &User{ID: 49935, Login: "geoffreywiseman", Type: "User"}
 
 	// When

client/transport.go

@@ -6,16 +6,16 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"os"
 	"strconv"
 	"time"
 
 	"golang.org/x/time/rate"
 )
 
 const (
-	// requestsPerSecond keeps the extension under GitHub's secondary limit of roughly 900 REST points per minute.
-	requestsPerSecond = 10
+	// RequestsPerSecond keeps the extension under GitHub's secondary limit of roughly 900 REST points per minute.
+	// It is exported so that callers can estimate how long a known number of requests will take.
+	RequestsPerSecond = 10
 	// requestBurst is the number of requests that may be issued back-to-back before the limiter starts pacing.
 	requestBurst = 10
 	// maxAttempts is the total number of times a request will be tried when GitHub reports it as rate limited.
@@ -44,20 +44,19 @@ type throttle struct {
 	notify func(wait time.Duration)
 }
 
-func newThrottle(next http.RoundTripper) *throttle {
+// newThrottle paces requests through next, and writes a notice to notices before each backoff.
+func newThrottle(next http.RoundTripper, notices io.Writer) *throttle {
 	return &throttle{
 		next:    next,
-		limiter: rate.NewLimiter(requestsPerSecond, requestBurst),
+		limiter: rate.NewLimiter(RequestsPerSecond, requestBurst),
 		now:     time.Now,
 		sleep:   sleepContext,
-		notify:  notifyStderr,
+		notify: func(wait time.Duration) {
+			_, _ = fmt.Fprintf(notices, "GitHub rate limit reached; waiting %s before retrying...\n", wait.Round(time.Second))
+		},
 	}
 }
 
-func notifyStderr(wait time.Duration) {
-	_, _ = fmt.Fprintf(os.Stderr, "GitHub rate limit reached; waiting %s before retrying...\n", wait.Round(time.Second))
-}
-
 // RoundTrip implements http.RoundTripper
 func (t *throttle) RoundTrip(req *http.Request) (*http.Response, error) {
 	ctx := req.Context()

client/transport_test.go

@@ -3,6 +3,7 @@ package client
 import (
 	"context"
 	"errors"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"strconv"
@@ -24,7 +25,7 @@ type throttleLog struct {
 // testThrottle returns a throttle that records requested sleeps and notifications instead of waiting or printing.
 func testThrottle(now time.Time) (*throttle, *throttleLog) {
 	log := &throttleLog{}
-	t := newThrottle(http.DefaultTransport)
+	t := newThrottle(http.DefaultTransport, io.Discard)
 	t.now = func() time.Time { return now }
 	t.sleep = func(_ context.Context, d time.Duration) error {
 		log.sleeps = append(log.sleeps, d)

main.go

@@ -13,13 +13,17 @@ import (
 	"time"
 
 	gogherrors "github.com/cli/go-gh/v2/pkg/api"
+	"github.com/cli/go-gh/v2/pkg/term"
 	"github.com/geoffreywiseman/gh-actions-usage/client"
 	"github.com/geoffreywiseman/gh-actions-usage/format"
 	"github.com/geoffreywiseman/gh-actions-usage/usage"
 )
 
 var gh client.Client
 
+// status is where progress of a collection is reported, apart from the report itself on stdout.
+var status = &statusWriter{Writer: os.Stderr}
+
 // monthLayout is the format of the --month flag, a year and month such as 2026-09.
 const monthLayout = "2006-01"
 
@@ -64,7 +68,7 @@ func (e InvalidMonthError) Error() string {
 func main() {
 	fmt.Printf("GitHub Actions Usage (%s)\n\n", getVersion())
 
-	gh = client.New()
+	gh = client.New(status)
 
 	now := time.Now()
 	cfg := &config{w: os.Stdout}
@@ -127,6 +131,7 @@ func tryDisplayCurrentRepo(cfg config) {
 		return
 	}
 	repoFlowUsage, err := collectUsage(cfg, []*client.Repository{repo})
+	status.separate()
 	if err != nil {
 		printError(cfg, "Error collecting usage", err)
 		return
@@ -149,6 +154,7 @@ func tryDisplayAllSpecified(cfg config, targets []string) {
 		targetsList = append(targetsList, list...)
 	}
 	repoFlowUsage, err := collectUsage(cfg, targetsList)
+	status.separate()
 	if err != nil {
 		printError(cfg, "Error collecting usage", err)
 		return
@@ -187,15 +193,19 @@ func parseMonth(value string, now time.Time) (time.Time, error) {
 	return month, nil
 }
 
-// collectUsage computes the usage of each repository's workflows for the configured billing period.
+// collectUsage computes the usage of each repository's workflows for the configured billing period. Progress
+// goes to stderr, so that the report on stdout stays clean for machine-readable formats.
 func collectUsage(cfg config, repos []*client.Repository) (client.RepoUsage, error) {
+	terminal := term.IsTerminal(os.Stderr)
 	collector := usage.Collector{
 		API:  &gh,
 		From: cfg.from,
 		To:   cfg.to,
 		Notify: func(message string) {
-			_, _ = fmt.Fprintln(os.Stderr, message)
+			_, _ = fmt.Fprintln(status, message)
 		},
+		ListingProgress: newProgressReporter(status, terminal, "repositories listed").update,
+		FetchProgress:   newProgressReporter(status, terminal, "commits fetched").update,
 	}
 	return collector.Collect(context.Background(), repos) //nolint:wrapcheck // caller prints the error as-is
 }
@@ -297,6 +307,9 @@ func mapOwner(repos repoMap, userName string) error {
 		list = make([]*client.Repository, 0)
 	}
 
+	// An owner with thousands of repositories takes a while to page through; say so on stderr, where the
+	// collector's progress goes too.
+	_, _ = fmt.Fprintf(status, "Listing repositories of %s...\n", user.Login)
 	ors, err := gh.GetAllRepositories(user)
 	if err != nil {
 		return fmt.Errorf("could not get repositories: %w", err)