Skip to content

refactor: Schedules delegates all scheduler HTTP to SchedulerClient#138

Closed
kowser-orkes wants to merge 1 commit into
feat/file-client-transfer-adaptersfrom
feature/single-scheduler-client
Closed

refactor: Schedules delegates all scheduler HTTP to SchedulerClient#138
kowser-orkes wants to merge 1 commit into
feat/file-client-transfer-adaptersfrom
feature/single-scheduler-client

Conversation

@kowser-orkes

Copy link
Copy Markdown
Contributor

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • WHOSUSING.md
  • Other (please describe):

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

Describe the new behavior from this PR, and why it's needed

  • One HTTP path instead of two: Schedules (the agent scheduling facade) no longer builds
    /scheduler/* requests itself — it now delegates to SchedulerClient, translating typed
    WorkflowSchedule/SaveScheduleRequest DTOs instead of raw JSON
  • SchedulerClient.pauseSchedule/resumeSchedule send PUT first (OSS Conductor's verb), fall
    back to GET and cache that verdict per client instance only on a 405 — same client now works
    unmodified against OSS Conductor, Orkes Conductor (GET-only today), and agentspan-embedded
    servers
  • SchedulerClient.getSchedule throws consistently on a miss on every server family (previously
    OSS's 200-empty vs Orkes' 404 diverged)
  • New: pauseSchedule(name, reason) (persisted as pausedReason where supported), nextRunTime
    (read) and cronSchedules on WorkflowSchedule/SaveScheduleRequest — additive,
    null-omitting so unused fields don't change existing wire bodies
  • Public API of Schedules/Schedule/ScheduleInfo is unchanged — Example99ScheduledAgent
    compiles with zero changes
  • Caught in review: the PUT→405→GET fallback recorded the 405 verdict but never actually sent
    the GET for that call, so pause/resume silently no-op'd on the triggering call — fixed to
    fall through to GET immediately instead of only on the next call

Issue #

Alternatives considered

Describe alternative implementation you have considered

  • Keep the two schedule HTTP paths separate — rejected, that's the duplication this PR removes
  • Always send one verb (GET-only or PUT-only) instead of PUT-first-with-cached-fallback —
    rejected, breaks portability across server families without per-deployment configuration
  • Drop the facade's cron-message InvalidCron exception sniffing — kept as-is for now (open
    product decision, revisit before GA)

Schedules no longer builds raw /scheduler/* requests itself — it holds a
SchedulerClient (OrkesSchedulerClient) and translates its typed responses
to/from Schedule/ScheduleInfo, keeping only agent-scoped naming
(prefix/unprefix), declarative reconcile, and runNow sugar (still via
WorkflowClient, untouched). Exception translation (404 -> NotFound,
400+"cron" -> InvalidCron, else AgentAPIException) stays in the facade;
SchedulerClient itself keeps throwing plain ConductorClientException so
standard-client users see no new exception types.

toSaveRequest/fromWorkflowSchedule now build/read the typed
SaveScheduleRequest/WorkflowSchedule DTOs instead of raw Maps. Public API
is unchanged (Example99ScheduledAgent compiles with zero changes).

Test fallout is purely mechanical: the four helper-level tests that
asserted on raw Map shapes now assert the same values through typed
getters. Every behavioural test (prefix/unprefix, validation, runNow*,
reconcile) is unmodified and still passes.
@kowser-orkes
kowser-orkes force-pushed the feature/single-scheduler-client branch from ea261e5 to f56504f Compare July 20, 2026 04:43
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.47619% with 29 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../conductoross/conductor/ai/schedule/Schedules.java 50.00% 23 Missing and 4 partials ⚠️
...es/conductor/client/http/OrkesSchedulerClient.java 0.00% 2 Missing ⚠️
Files with missing lines Coverage Δ Complexity Δ
...ava/io/orkes/conductor/client/SchedulerClient.java 100.00% <100.00%> (ø) 1.00 <1.00> (?)
...orkes/conductor/client/http/SchedulerResource.java 36.13% <100.00%> (+36.13%) 13.00 <9.00> (+13.00)
...es/conductor/client/http/OrkesSchedulerClient.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../conductoross/conductor/ai/schedule/Schedules.java 42.30% <50.00%> (+4.43%) 20.00 <5.00> (-4.00) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kowser-orkes
kowser-orkes changed the base branch from main to feat/file-client-transfer-adapters July 20, 2026 05:31
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.

1 participant