Skip to content

feat: [Cadence Schedules] Add schedule core types and enums#7745

Merged
abhishekj720 merged 1 commit intomasterfrom
schedule-core-types
Feb 25, 2026
Merged

feat: [Cadence Schedules] Add schedule core types and enums#7745
abhishekj720 merged 1 commit intomasterfrom
schedule-core-types

Conversation

@abhishekj720
Copy link
Contributor

@abhishekj720 abhishekj720 commented Feb 25, 2026

What changed?
Added internal Go types for Cadence Schedules (enums, core structs) mirroring api/v1/schedule.proto.

Why?
To support the new Cadence Schedules feature (pause/resume, catch-up, backfill), we need internal Go types to represent schedule entities within the server codebase. These types mirror the IDL definitions and include standard helper methods (Getters, String/Enum marshalling) required for internal usage. This serves as the foundation for the scheduler workflow and API implementation.
Proto definitions: cadence-workflow/cadence-idl#246

How did you test it?
Ran unit tests for enum marshalling/unmarshalling:
go test -v ./common/types/ -run TestSchedule

Potential risks
N/A - Purely additive type definitions; not yet wired into any active code paths.

Release notes
Added core internal types for Cadence Schedules.

Documentation Changes
N/A


Reviewer Validation

PR Description Quality (check these before reviewing code):

  • "What changed" provides a clear 1-2 line summary
    • Project Issue is linked
  • "Why" explains the full motivation with sufficient context
  • Testing is documented:
    • Unit test commands are included (with exact go test invocation)
    • Integration test setup/commands included (if integration tests were run)
    • Canary testing details included (if canary was mentioned)
  • Potential risks section is thoughtfully filled out (or legitimately N/A)
  • Release notes included if this completes a user-facing feature
  • Documentation needs are addressed (or noted if uncertain)

if err != nil {
return fmt.Errorf("unknown enum value %q for %q: %v", s, "ScheduleCatchUpPolicy", err)
}
*e = ScheduleCatchUpPolicy(val)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to error instead of accepting arbitrary numbers? I don't recall the behavior of other types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matches existing convention: all enums in common/types accept arbitrary numeric values for forward-compatibility.
For example:

if err != nil {

TaskStartToCloseTimeoutSeconds *int32 `json:"taskStartToCloseTimeoutSeconds,omitempty"`
RetryPolicy *RetryPolicy `json:"retryPolicy,omitempty"`
Memo *Memo `json:"-"`
SearchAttributes *SearchAttributes `json:"-"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would advocate for using value types for all these fields as well but this seems aligned with how we handle these fields everywhere else.

@abhishekj720 abhishekj720 force-pushed the schedule-core-types branch 2 times, most recently from e9da54d to 899ddf4 Compare February 25, 2026 18:52
@abhishekj720 abhishekj720 enabled auto-merge (squash) February 25, 2026 18:57
Signed-off-by: abhishek.jha <abhishek.jha@uber.com>
@gitar-bot
Copy link

gitar-bot bot commented Feb 25, 2026

Code Review ✅ Approved 2 resolved / 2 findings

Clean foundational type definitions following established codebase patterns. Both previous findings (missing BackfillInfo getters and enum String() format mismatch) have been resolved.

✅ 2 resolved
Bug: String() format for unknown enum values mismatches tests and convention

📄 common/types/schedule.go:61 📄 common/types/schedule.go:115 📄 common/types/schedule_test.go:72 📄 common/types/schedule_test.go:140
Both ScheduleOverlapPolicy.String() (line 61) and ScheduleCatchUpPolicy.String() (line 115) use fmt.Sprintf("%d", int32(e)) for unknown/out-of-range values, producing bare numbers like "99".

This causes two problems:

  1. Tests will fail: The test at schedule_test.go:72 expects "ScheduleOverlapPolicy(99)" and the test at schedule_test.go:140 expects "ScheduleCatchUpPolicy(99)", but the implementation returns "99" for both.

  2. Inconsistent with codebase convention: Every other enum in the common/types package uses the TypeName(%d) format — e.g., TaskListKind(%d), ArchivalStatus(%d), DomainStatus(%d), EventType(%d), etc. (42 instances across shared.go, matching.go, replicator.go). The bare number format breaks this convention and makes debugging harder since the type information is lost in log output.

Quality: BackfillInfo missing GetStartTime() and GetEndTime() getters

📄 common/types/schedule.go:342-348
BackfillInfo has 5 fields (BackfillID, StartTime, EndTime, RunsCompleted, RunsTotal) but only 3 getter methods are defined (GetBackfillID, GetRunsCompleted, GetRunsTotal). The GetStartTime() and GetEndTime() getters are missing, breaking the nil-safe getter pattern that every other struct in this file and the common/types/ package follows consistently.

While these types aren't wired into active code paths yet, consumers of BackfillInfo will need these getters for nil-safe access and for consistency with the rest of the type system.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@abhishekj720 abhishekj720 merged commit 32814ea into master Feb 25, 2026
75 checks passed
@abhishekj720 abhishekj720 deleted the schedule-core-types branch February 25, 2026 20:00
abhishekj720 added a commit that referenced this pull request Feb 26, 2026
)

**What changed?**
- Add request/response types for all Schedule CRUD and lifecycle APIs
(Create, Describe, Update, Delete, Pause, Unpause, List, Backfill) in
`common/types/schedule_service.go`
- Add `ScheduleListEntry` for paginated list responses
- Include nil-safe getters for every field on every type, consistent
  with existing patterns in `common/types/shared.go`
- Add `schedule_service_test.go` with nil-receiver and value getter
tests

**Why?**
To support the new Cadence Schedules feature (pause/resume, catch-up,
backfill), we need internal Go types to represent schedule entities
within the server codebase. These types mirror the IDL definitions. This
serves as the foundation for the scheduler workflow and API
implementation.

Builds on [#7745](#7745)
which introduced the core schedule types and enums.

**How did you test it?**
Added unit tests and ran go test ./.. on the added files 

**Potential risks**
N/A

**Release notes**
Added request/response types for cadence schedules

**Documentation Changes**
N/A

---

## Reviewer Validation

**PR Description Quality** (check these before reviewing code):

- [ ] **"What changed"** provides a clear 1-2 line summary
  - [ ] Project Issue is linked
- [ ] **"Why"** explains the full motivation with sufficient context
- [ ] **Testing is documented:**
- [ ] Unit test commands are included (with exact `go test` invocation)
- [ ] Integration test setup/commands included (if integration tests
were run)
  - [ ] Canary testing details included (if canary was mentioned)
- [ ] **Potential risks** section is thoughtfully filled out (or
legitimately N/A)
- [ ] **Release notes** included if this completes a user-facing feature
- [ ] **Documentation** needs are addressed (or noted if uncertain)

Signed-off-by: abhishek.jha <abhishek.jha@uber.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants