Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,14 @@ linters:
- linters:
- noctx
path: _test\.go
# misspell: "cancelled" is the canonical stored status value in
# pkg/ladder (RunStatusCancelled, TrancheStatusCancelled); allow the UK
# spelling in the files that carry it.
# misspell: pkg/ladder writes "canceled" but still reads the legacy
# "cancelled" that rows carried before #554, and keeps the deprecated
# RunStatusCancelled/TrancheStatusCancelled aliases; allow the UK spelling
# in the files that carry it.
- linters:
- misspell
path: pkg/ladder/(store|types_test)\.go
text: "cancelled"
path: pkg/ladder/(store|types_test|canceled_spelling_test)\.go
text: "(?i)cancelled"
paths:
- third_party$
- builtin$
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
`cache.PurchaseCommitment` now refuses before any API call with an error
wrapping `common.ErrRefusedBeforePurchase`. Consumers that buy an Azure
`ServiceCache` recommendation must route it to managedredis (#538).
- Ladder run and tranche statuses now write the US spelling `canceled`
(`RunStatusCanceled`, `TrancheStatusCanceled`). Rows holding the legacy
`cancelled` still validate, count as terminal, and parse to the new
constants through `ParseRunStatus` and `ParseTrancheStatus`. The
`RunStatusCancelled` and `TrancheStatusCancelled` names remain as
deprecated aliases with the new value. Consumers that compare raw stored
strings or filter on `cancelled` in SQL must accept both spellings.
- `ListTargetOfferings` now walks up to 50 pages (about 5000 offerings)
instead of 10, and returns an error wrapping the new
`ec2.ErrTargetOfferingsTruncated` (with no list) when that bound is hit,
Expand Down
42 changes: 42 additions & 0 deletions pkg/ladder/canceled_spelling_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package ladder

import "testing"

// #554: writers emit the US spelling "canceled"; rows written before keep the
// legacy "cancelled" and still parse, validate and count as terminal.

func TestCanceledStatuses_WriteTheUSSpelling(t *testing.T) {
t.Parallel()
if RunStatusCanceled != "canceled" || TrancheStatusCanceled != "canceled" {
t.Fatalf("writers must emit \"canceled\": run %q, tranche %q", RunStatusCanceled, TrancheStatusCanceled)
}
if RunStatusCancelled != RunStatusCanceled || TrancheStatusCancelled != TrancheStatusCanceled {
t.Fatal("the deprecated Cancelled aliases must equal the Canceled constants")
}
}

func TestCanceledStatuses_LegacySpellingParsesToCanceled(t *testing.T) {
t.Parallel()
for _, in := range []string{"canceled", "cancelled"} {
run, err := ParseRunStatus(in)
if err != nil || run != RunStatusCanceled {
t.Errorf("ParseRunStatus(%q) = %q, %v; want %q", in, run, err, RunStatusCanceled)
}
tr, err := ParseTrancheStatus(in)
if err != nil || tr != TrancheStatusCanceled {
t.Errorf("ParseTrancheStatus(%q) = %q, %v; want %q", in, tr, err, TrancheStatusCanceled)
}
if err := RunStatus(in).Validate(); err != nil {
t.Errorf("RunStatus(%q).Validate() = %v; a stored legacy row must still validate", in, err)
}
if err := TrancheStatus(in).Validate(); err != nil {
t.Errorf("TrancheStatus(%q).Validate() = %v", in, err)
}
if !RunStatus(in).IsTerminal() {
t.Errorf("RunStatus(%q).IsTerminal() = false", in)
}
if TrancheStatus(in).allowsFiredAt() {
t.Errorf("TrancheStatus(%q) must not allow FiredAt", in)
}
}
}
40 changes: 32 additions & 8 deletions pkg/ladder/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,40 +17,52 @@ const (
RunStatusExecuting RunStatus = "executing"
RunStatusCompleted RunStatus = "completed"
RunStatusFailed RunStatus = "failed"
RunStatusCancelled RunStatus = "cancelled"
RunStatusCanceled RunStatus = "canceled"
RunStatusExpired RunStatus = "expired"

// Deprecated: use RunStatusCanceled. Kept so existing callers compile; it
// writes the new spelling.
RunStatusCancelled = RunStatusCanceled

// runStatusLegacyCancelled is the spelling rows carried before #554. It is
// read, never written.
runStatusLegacyCancelled RunStatus = "cancelled"
)

// Validate returns an error when s is not a recognized RunStatus.
func (s RunStatus) Validate() error {
switch s {
case RunStatusPlanned, RunStatusAwaitingApproval, RunStatusApproved,
RunStatusExecuting, RunStatusCompleted, RunStatusFailed,
RunStatusCancelled, RunStatusExpired:
RunStatusCanceled, runStatusLegacyCancelled, RunStatusExpired:
return nil
}
return fmt.Errorf("unknown run status %q", s)
}

// IsTerminal reports whether s is a terminal run status: RunStatusCompleted,
// RunStatusFailed, RunStatusCancelled, or RunStatusExpired. Terminal runs are
// RunStatusFailed, RunStatusCanceled (either spelling), or RunStatusExpired. Terminal runs are
// the only ones allowed to carry a CompletedAt timestamp (see
// RunRecord.Validate).
func (s RunStatus) IsTerminal() bool {
switch s {
case RunStatusCompleted, RunStatusFailed, RunStatusCancelled, RunStatusExpired:
case RunStatusCompleted, RunStatusFailed, RunStatusCanceled, runStatusLegacyCancelled, RunStatusExpired:
return true
}
return false
}

// ParseRunStatus converts s into a RunStatus, returning a descriptive error
// when s is not a recognized value.
// when s is not a recognized value. The legacy "cancelled" reads as
// RunStatusCanceled.
func ParseRunStatus(s string) (RunStatus, error) {
st := RunStatus(s)
if err := st.Validate(); err != nil {
return "", err
}
if st == runStatusLegacyCancelled {
return RunStatusCanceled, nil
}
return st, nil
}

Expand All @@ -61,15 +73,23 @@ const (
TrancheStatusScheduled TrancheStatus = "scheduled"
TrancheStatusFired TrancheStatus = "fired"
TrancheStatusCompleted TrancheStatus = "completed"
TrancheStatusCancelled TrancheStatus = "cancelled"
TrancheStatusCanceled TrancheStatus = "canceled"
TrancheStatusFailed TrancheStatus = "failed"

// Deprecated: use TrancheStatusCanceled. Kept so existing callers compile;
// it writes the new spelling.
TrancheStatusCancelled = TrancheStatusCanceled

// trancheStatusLegacyCancelled is the spelling rows carried before #554.
// It is read, never written.
trancheStatusLegacyCancelled TrancheStatus = "cancelled"
)

// Validate returns an error when s is not a recognized TrancheStatus.
func (s TrancheStatus) Validate() error {
switch s {
case TrancheStatusScheduled, TrancheStatusFired, TrancheStatusCompleted,
TrancheStatusCancelled, TrancheStatusFailed:
TrancheStatusCanceled, trancheStatusLegacyCancelled, TrancheStatusFailed:
return nil
}
return fmt.Errorf("unknown tranche status %q", s)
Expand All @@ -88,12 +108,16 @@ func (s TrancheStatus) allowsFiredAt() bool {
}

// ParseTrancheStatus converts s into a TrancheStatus, returning a
// descriptive error when s is not a recognized value.
// descriptive error when s is not a recognized value. The legacy "cancelled"
// reads as TrancheStatusCanceled.
func ParseTrancheStatus(s string) (TrancheStatus, error) {
st := TrancheStatus(s)
if err := st.Validate(); err != nil {
return "", err
}
if st == trancheStatusLegacyCancelled {
return TrancheStatusCanceled, nil
}
return st, nil
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/ladder/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestRunStatusIsTerminal(t *testing.T) {
{RunStatusExecuting, false},
{RunStatusCompleted, true},
{RunStatusFailed, true},
{RunStatusCancelled, true},
{RunStatusCanceled, true},
{RunStatusExpired, true},
}
for _, c := range cases {
Expand Down Expand Up @@ -86,9 +86,9 @@ func TestRunRecordValidate(t *testing.T) {
wantErr: false,
},
{
name: "RunStatusCancelled with CompletedAt is valid",
name: "RunStatusCanceled with CompletedAt is valid",
mutate: func(r *RunRecord) {
r.Status = RunStatusCancelled
r.Status = RunStatusCanceled
r.CompletedAt = &now
},
wantErr: false,
Expand Down Expand Up @@ -229,9 +229,9 @@ func TestTrancheValidate(t *testing.T) {
wantErr: true,
},
{
name: "TrancheStatusCancelled with FiredAt is invalid",
name: "TrancheStatusCanceled with FiredAt is invalid",
mutate: func(tr *Tranche) {
tr.Status = TrancheStatusCancelled
tr.Status = TrancheStatusCanceled
tr.FiredAt = &now
},
wantErr: true,
Expand Down
10 changes: 6 additions & 4 deletions pkg/ladder/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func TestRunStatusValidate(t *testing.T) {
{RunStatusExecuting, false},
{RunStatusCompleted, false},
{RunStatusFailed, false},
{RunStatusCancelled, false},
{RunStatusCanceled, false},
{RunStatusExpired, false},
{"unknown-status", true},
{"", true},
Expand All @@ -200,7 +200,7 @@ func TestTrancheStatusValidate(t *testing.T) {
{TrancheStatusScheduled, false},
{TrancheStatusFired, false},
{TrancheStatusCompleted, false},
{TrancheStatusCancelled, false},
{TrancheStatusCanceled, false},
{TrancheStatusFailed, false},
{"unknown-tranche-status", true},
{"", true},
Expand Down Expand Up @@ -490,7 +490,8 @@ func TestParseRunStatus(t *testing.T) {
{"executing", RunStatusExecuting, false},
{"completed", RunStatusCompleted, false},
{"failed", RunStatusFailed, false},
{"cancelled", RunStatusCancelled, false},
{"canceled", RunStatusCanceled, false},
{"cancelled", RunStatusCanceled, false}, // legacy rows read as canceled (#554)
{"expired", RunStatusExpired, false},
{"bogus", "", true},
{"", "", true},
Expand Down Expand Up @@ -523,7 +524,8 @@ func TestParseTrancheStatus(t *testing.T) {
{"scheduled", TrancheStatusScheduled, false},
{"fired", TrancheStatusFired, false},
{"completed", TrancheStatusCompleted, false},
{"cancelled", TrancheStatusCancelled, false},
{"canceled", TrancheStatusCanceled, false},
{"cancelled", TrancheStatusCanceled, false}, // legacy rows read as canceled (#554)
{"failed", TrancheStatusFailed, false},
{"bogus", "", true},
{"", "", true},
Expand Down
Loading