feat(schedule): support creating schedule in paused state - #1081
feat(schedule): support creating schedule in paused state#1081abhishekj720 wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (76.31%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage.
... and 12 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Add ScheduleInitialState input type and a five-arg createSchedule() overload that accepts it. Passing ScheduleInitialState with paused=true creates the schedule already paused, removing the need for a subsequent PauseSchedule call. pausedAt is server-populated and intentionally excluded from this type. Also updates the src/main/idls submodule to d6d4d81 (adds state field to CreateScheduleRequest in shared.thrift) and adds the corresponding state field to the generated CreateScheduleRequest Java class. Signed-off-by: abhishek.jha <abhishek.jha@uber.com>
d981c5f to
7089f29
Compare
…e, toString and partial pause-info branches Signed-off-by: abhishek.jha <abhishek.jha@uber.com>
| .setAction(toThriftAction(action)) | ||
| .setPolicies(toThriftPolicies(policies)); | ||
| if (initialState != null) { | ||
| request.setState(toThriftInitialState(initialState)); |
There was a problem hiding this comment.
We set state on the request here, but it never gets serialized. On the gRPC path, WorkflowServiceGrpc.CreateSchedule sends the request via RequestMapper.createScheduleRequest(request), there's no setState(...) and t.getState() is never read. So state is dropped and the schedule comes back un-paused, no error.
| // --- initialState --- | ||
|
|
||
| @Test | ||
| public void createSchedule_initialState_pausedWithReasonAndPausedBy() throws Exception { |
There was a problem hiding this comment.
These assert ARE before RequestMapper runs — that's why they stay green even though state gets dropped on the way to the proto. Could we add one that runs createScheduleRequest and checks the resulting api.v1.CreateScheduleRequest (getState().getPaused() etc.)? That would've caught this.
…t proto RequestMapper.createScheduleRequest was missing the state field, causing initialState to be silently dropped on the gRPC path. Add TypeMapper overloads for the thrift→proto direction (schedulePauseInfo and scheduleState) and wire state into the proto builder. Add a RequestMapper-level test that verifies state and pauseInfo survive the full thrift→proto conversion. Signed-off-by: abhishek.jha <abhishek.jha@uber.com>
…eduleRequest) The IDL commit adds ScheduleState state = 8 to CreateScheduleRequest proto, which the gRPC mapper now relies on to serialize initial pause state. Signed-off-by: abhishek.jha <abhishek.jha@uber.com>
Code Review ✅ ApprovedAdds ScheduleInitialState and an overloaded createSchedule method to support creating schedules in a paused state, along with comprehensive unit tests and IDL submodule updates. No issues found. OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
2 participants
Summary
ScheduleInitialState, an input-side type capturingpaused,pauseReason, andpausedBy(pausedAtis excluded as it is server-populated)createSchedule()overload acceptingScheduleInitialState; the existing four-arg overload delegates to it withnull, preserving backward compatibilitytoThriftInitialState()helper inScheduleClientImplmappingScheduleInitialState→com.uber.cadence.ScheduleStatesrc/main/idlssubmodule tod6d4d81which adds thestatefield toCreateScheduleRequestinshared.thriftstatefield to the generatedCreateScheduleRequestJava classTest plan
./gradlew compileJava -x generateProto: compiles cleanly./gradlew test --tests "com.uber.cadence.internal.sync.ScheduleClientImplTest": all tests pass (including four newinitialStatetests)