cli: dedup mirror-active filter and cell-fallback logging

21ff7f2·

Soph·2w ago·3 files·+33 added/-21 removed

Cleanups from a simplify pass over the activity/recap routing:

- Extract isActiveMirror(coreapi.Mirror) — the archived + failed/suspended
placement filter that firstActiveRepoID and distinctActiveClusterHosts
each spelled out — so "can this placement serve the repo" has one home;
a new non-serving status now only needs updating there.
- Extract logCellClientFallback for the two cell→data-API call sites,
replacing the duplicated inverse-condition debug log. It also drops the
noise for the expected not-logged-in case (not just no-cell-yet), and
removes the awkward if/else-with-empty-branch in newRecapClient.

No behaviour change.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com

Sessions

8d8cf40beadeView transcript

Changes

3

• cmd/entire/cli

• Mentireapi_client.go+13/-7

• Mexperts_cell_target.go+13/-6

• Mrecap.go+7/-8

27 unmodified lines

func runAuthenticatedActivityAPI(ctx context.Context, errW io.Writer, insecureHTTP bool, fn func(context.Context, *api.Client) error) error {
    client, err := auth.NewEntireAPICellClient(ctx, insecureHTTP, nil)
    if err != nil {
        if !errors.Is(err, auth.ErrNoCellForJurisdiction) {
            logging.Debug(ctx, "activity/recap: entire-api cell client unavailable, using data API", "error", err.Error())
        }
        logCellClientFallback(ctx, err)
        return runAuthenticatedDataAPI(ctx, errW, insecureHTTP, fn)
    }
    return fn(ctx, client)
}

// logCellClientFallback records, at debug, that an activity/recap command fell
// back from the entire-api cell to the data API. The expected cases — the
// region has no cell yet, or the caller isn't logged in — aren't logged: they
// are normal during rollout and on first use, not diagnosable failures.
func logCellClientFallback(ctx context.Context, err error) {
    if errors.Is(err, auth.ErrNoCellForJurisdiction) || errors.Is(err, auth.ErrNotLoggedIn) {
        return
    }
    logging.Debug(ctx, "activity/recap: entire-api cell client unavailable, using data API", "error", err.Error())
}

// forgeToMirrorProvider maps a gitremote forge identifier (e.g. "gh") to the
// upstream provider the control plane records mirrors under (e.g. "github").
// entire-api routing only supports GitHub mirrors today.
// and failed/suspended placements are skipped — they can't answer for the repo.
func firstActiveRepoID(mirrors []coreapi.Mirror) string {
    for i := range mirrors {
        if mirrors[i].IsArchived.Or(false) {
            continue
        }
        if st := mirrors[i].Status.Or(coreapi.MirrorStatusReady); st == coreapi.MirrorStatusFailed || st == coreapi.MirrorStatusSuspended {
            if !isActiveMirror(mirrors[i]) {
                continue
            }
            if id := strings.TrimSpace(mirrors[i].MirrorId); id != "" {
""

Mcmd/entire/cli/entireapi_client.go+13/-7

121 unmodified lines

// isActiveMirror reports whether a mirror placement can currently serve the
// repo: not archived, and not in a failed/suspended clone state. An unset status
// is treated as active (older data). Shared by every caller that must ignore
// placements a cell can't answer for.
func isActiveMirror(m coreapi.Mirror) bool {
    if m.IsArchived.Or(false) {
        return false
    }
    st := m.Status.Or(coreapi.MirrorStatusReady)
    return st != coreapi.MirrorStatusFailed && st != coreapi.MirrorStatusSuspended
}

// distinctActiveClusterHosts returns the set of cluster hosts a repo is actively
// serviced on: excluding archived placements and unhealthy ones (failed /
// suspended clone status), since those cells can't answer experts for the repo.
func distinctActiveClusterHosts(mirrors []coreapi.Mirror) []string {
    seen := make(map[string]string, len(mirrors))
    for _, m := range mirrors {
        if m.IsArchived.Or(false) {
            continue
        }
        // Treat unset status as active (older data); only failed/suspended
        // placements can't serve experts for the repo.
        if st := m.Status.Or(coreapi.MirrorStatusReady); st == coreapi.MirrorStatusFailed || st == coreapi.MirrorStatusSuspended {
            if !isActiveMirror(m) {
                continue
            }
        host := strings.TrimSpace(m.ClusterHost)
}

Mcmd/entire/cli/experts_cell_target.go+13/-6

17 unmodified lines

// the caller isn't logged in — recap tolerates the latter, rendering and letting
// the server answer 401 rather than hard-failing. Every other failure surfaces.
func newRecapClient(ctx context.Context, insecureHTTP bool) (*api.Client, string, error) {
    if client, err := auth.NewEntireAPICellClient(ctx, insecureHTTP, nil); err == nil {
        return client, currentRepoID(ctx), nil
    } else if !errors.Is(err, auth.ErrNoCellForJurisdiction) {
        // Best-effort upgrade: fall back to the data API on any cell failure. Its
        // path below tolerates a missing login (renders, lets the server 401), so
        // the not-logged-in case keeps working; log non-obvious failures.
        logging.Debug(ctx, "recap: entire-api cell client unavailable, using data API", "error", err.Error())
        // Best-effort upgrade: on any cell failure fall back to the data API path
        // below, which tolerates a missing login (renders, lets the server 401) so
        // the not-logged-in case keeps working.
        cellClient, cellErr := auth.NewEntireAPICellClient(ctx, insecureHTTP, nil)
        if cellErr == nil {
            return cellClient, currentRepoID(ctx), nil
        }
        logCellClientFallback(ctx, cellErr)

Mcmd/entire/cli/recap.go+7/-8