feat: add LocalRunService for reporting locally-orchestrated runs - #7737
Conversation
There was a problem hiding this comment.
🟡 Not ready to approve
The new ArtifactType values widen existing APIs unless request validations are tightened (and lint suppression should be narrowed), which can unintentionally expand supported behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR introduces new IDL surfaces for representing and reporting locally-orchestrated runs (outside Flyte’s dataplane), including a new LocalRunService that mirrors RunService read/watch APIs and new DataProxyService RPCs for control-plane-local metadata artifact upload/download, plus a new RunSource value to tag these runs.
Changes:
- Added
flyteidl2.workflow.LocalRunService(create + report APIs, and RunService-mirroring read/watch/list APIs) and generated clients/stubs across Go/TS/Python. - Extended
flyteidl2.dataproxy.DataProxyServicewith local-run artifact RPCs (UploadMetadata,GetLocalActionData,CreateLocalDownloadLink) and addedArtifactTypevaluesINPUTS/OUTPUTS. - Added
RunSource.RUN_SOURCE_LOCALand updated buf lint ignores to allow intentional request/response message reuse.
File summaries
| File | Description |
|---|---|
| gen/ts/flyteidl2/workflow/local_run_service_pb.ts | New TS generated types/service descriptor for LocalRunService. |
| gen/ts/flyteidl2/dataproxy/dataproxy_service_pb.ts | TS generated updates for new dataproxy RPCs/messages and ArtifactType values. |
| gen/rust/src/flyteidl2.dataproxy.tonic.rs | Rust tonic client/server stubs updated for new dataproxy RPCs. |
| gen/python/flyteidl2/workflow/run_definition_pb2.pyi | Python typing updates for new RunSource.RUN_SOURCE_LOCAL. |
| gen/python/flyteidl2/workflow/local_run_service_pb2.pyi | New Python typing stubs for LocalRunService messages. |
| gen/python/flyteidl2/workflow/local_run_service_pb2.py | New Python protobuf module for LocalRunService. |
| gen/python/flyteidl2/workflow/local_run_service_pb2_grpc.py | New Python gRPC stubs for LocalRunService. |
| gen/python/flyteidl2/dataproxy/dataproxy_service_pb2.pyi | Python typing updates for new dataproxy messages/enums. |
| gen/python/flyteidl2/dataproxy/dataproxy_service_pb2.py | Python protobuf module updated for new dataproxy RPCs/messages and enum values. |
| gen/python/flyteidl2/dataproxy/dataproxy_service_pb2_grpc.py | Python gRPC stubs updated for new dataproxy RPCs. |
| gen/python/flyteidl2/dataproxy/dataproxy_service_connect.py | Python Connect bindings updated for new dataproxy RPCs. |
| gen/go/gateway/flyteidl2/workflow/local_run_service.swagger.json | New gateway swagger artifact for LocalRunService. |
| gen/go/flyteidl2/workflow/workflowconnect/local_run_service.connect.go | New Go Connect client/handler for LocalRunService. |
| gen/go/flyteidl2/workflow/run_definition.pb.go | Go generated enum update for RunSource_RUN_SOURCE_LOCAL. |
| gen/go/flyteidl2/workflow/local_run_service.pb.validate.go | New Go validation stubs for LocalRunService messages. |
| gen/go/flyteidl2/workflow/local_run_service_grpc.pb.go | New Go gRPC client/server stubs for LocalRunService. |
| gen/go/flyteidl2/dataproxy/dataproxyconnect/dataproxy_service.connect.go | Go Connect client/handler updated for new dataproxy RPCs. |
| gen/go/flyteidl2/dataproxy/dataproxy_service.pb.validate.go | Go validation stubs updated for new dataproxy messages. |
| gen/go/flyteidl2/dataproxy/dataproxy_service_grpc.pb.go | Go gRPC client/server stubs updated for new dataproxy RPCs. |
| flyteidl2/workflow/run_definition.proto | Adds RUN_SOURCE_LOCAL to tag locally-orchestrated runs. |
| flyteidl2/workflow/local_run_service.proto | New LocalRunService proto + request/response messages for create/report. |
| flyteidl2/dataproxy/dataproxy_service.proto | Adds local-run dataproxy RPCs, new messages, and new ArtifactType values. |
| buf.yaml | Updates lint ignores to allow intentional request/response message reuse for local-run APIs. |
Review details
Files not reviewed (13)
- gen/go/flyteidl2/dataproxy/dataproxy_service.pb.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxy_service.pb.validate.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxy_service_grpc.pb.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxyconnect/dataproxy_service.connect.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service.pb.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service.pb.validate.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service_grpc.pb.go: Generated file
- gen/go/flyteidl2/workflow/run_definition.pb.go: Generated file
- gen/go/flyteidl2/workflow/workflowconnect/local_run_service.connect.go: Generated file
- gen/go/flyteidl2/workflow/workflowconnect/mocks/mocks.go: Generated file
- gen/python/flyteidl2/dataproxy/dataproxy_service_pb2.py: Generated file
- gen/python/flyteidl2/workflow/local_run_service_pb2.py: Generated file
- gen/python/flyteidl2/workflow/run_definition_pb2.py: Generated file
- Files reviewed: 25/53 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| - flyteidl2/dataproxy/dataproxy_service.proto | ||
| - flyteidl2/workflow/local_run_service.proto | ||
| - flyteidl2/workflow/run_service.proto |
| ArtifactType artifact_type = 1 [(buf.validate.field).enum = { | ||
| not_in: [0] | ||
| }]; |
| // ARTIFACT_TYPE_INPUTS refers to an action's serialized inputs (inputs.pb). | ||
| ARTIFACT_TYPE_INPUTS = 3; | ||
| // ARTIFACT_TYPE_OUTPUTS refers to an action's serialized outputs (outputs.pb). | ||
| ARTIFACT_TYPE_OUTPUTS = 4; |
c10a67c to
390e957
Compare
There was a problem hiding this comment.
🟡 Not ready to approve
The new dataproxy request types/documentation allow contradictory combinations (e.g., artifact type vs target identifier), which should be enforced via validation or narrowed enum constraints to avoid ambiguous client/server behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (13)
- gen/go/flyteidl2/dataproxy/dataproxy_service.pb.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxy_service.pb.validate.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxy_service_grpc.pb.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxyconnect/dataproxy_service.connect.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service.pb.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service.pb.validate.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service_grpc.pb.go: Generated file
- gen/go/flyteidl2/workflow/run_definition.pb.go: Generated file
- gen/go/flyteidl2/workflow/workflowconnect/local_run_service.connect.go: Generated file
- gen/go/flyteidl2/workflow/workflowconnect/mocks/mocks.go: Generated file
- gen/python/flyteidl2/dataproxy/dataproxy_service_pb2.py: Generated file
- gen/python/flyteidl2/workflow/local_run_service_pb2.py: Generated file
- gen/python/flyteidl2/workflow/run_definition_pb2.py: Generated file
Suppressed comments (3)
flyteidl2/dataproxy/dataproxy_service.proto:306
- CreateLocalDownloadLinkRequest takes an
action_attempt_id, butartifact_typecurrently allows any non-zero value (including INPUTS and CODE_BUNDLE). That conflicts with the message comment (“action attempt whose artifact is to be downloaded”) and likely yields ambiguous/unsupported combinations.
message CreateLocalDownloadLinkRequest {
// ArtifactType is the type of artifact to download.
// +required
ArtifactType artifact_type = 1 [(buf.validate.field).enum = {
not_in: [0]
}];
flyteidl2/dataproxy/dataproxy_service.proto:279
- UploadMetadataRequest documents that INPUTS targets an action while OUTPUTS/REPORT target an action attempt, but the current schema only requires that some
targetoneof is set. This allows invalid combinations (e.g., artifact_type=INPUTS with action_attempt_id) that consumers/servers will have to special-case.
// UploadMetadataRequest specifies the request for the UploadMetadata API. The data proxy service
// derives the storage path from the authenticated org and the target action, following the local
// run metadata layout: <prefix>/<org>/<project>/<domain>/<run>/<action>/inputs.pb for inputs and
// <prefix>/<org>/<project>/<domain>/<run>/<action>/<attempt>/{outputs.pb,report.html} otherwise.
message UploadMetadataRequest {
// ArtifactType is the type of artifact to upload. INPUTS targets an action; OUTPUTS and REPORT
// target an action attempt.
ArtifactType artifact_type = 1 [(buf.validate.field).enum = {
in: [
1,
3,
4
]
}];
// Target identifies the action (INPUTS) or action attempt (OUTPUTS / REPORT) the artifact
// belongs to.
oneof target {
option (buf.validate.oneof).required = true;
common.ActionIdentifier action_id = 2;
common.ActionAttemptIdentifier action_attempt_id = 3;
}
flyteidl2/dataproxy/dataproxy_service.proto:317
- Typo in comment: "Reponse" -> "Response".
// Reponse message for tailing logs.
- Files reviewed: 16/32 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Adds flyteidl2.workflow.LocalRunService: clients that orchestrate actions outside the platform (e.g. on a user's machine) register a run and report action state, while the read/watch surface reuses RunService's request and response messages so existing clients and UIs work with only a service swap. Also adds the control-plane-direct dataproxy surface for local run data: UploadMetadata (signed PUT URLs for inputs.pb/outputs.pb/report.html), GetLocalActionData and CreateLocalDownloadLink, plus ArtifactType INPUTS/OUTPUTS values and RunSource.RUN_SOURCE_LOCAL. Signed-off-by: Haytham Abuelfutuh <haytham@union.ai> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
390e957 to
b31e188
Compare
There was a problem hiding this comment.
🟡 Not ready to approve
The new dataproxy IDL adds request shapes where important cross-field invariants (artifact type vs target scope/presence) are not enforced, making the API easy to misuse and harder to implement correctly.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (13)
- gen/go/flyteidl2/dataproxy/dataproxy_service.pb.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxy_service.pb.validate.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxy_service_grpc.pb.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxyconnect/dataproxy_service.connect.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service.pb.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service.pb.validate.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service_grpc.pb.go: Generated file
- gen/go/flyteidl2/workflow/run_definition.pb.go: Generated file
- gen/go/flyteidl2/workflow/workflowconnect/local_run_service.connect.go: Generated file
- gen/go/flyteidl2/workflow/workflowconnect/mocks/mocks.go: Generated file
- gen/python/flyteidl2/dataproxy/dataproxy_service_pb2.py: Generated file
- gen/python/flyteidl2/workflow/local_run_service_pb2.py: Generated file
- gen/python/flyteidl2/workflow/run_definition_pb2.py: Generated file
Suppressed comments (3)
flyteidl2/dataproxy/dataproxy_service.proto:306
- CreateLocalDownloadLinkRequest is scoped to an action attempt, but artifact_type currently allows any non-zero ArtifactType (including INPUTS). Since INPUTS are stored at the action level (and this request only provides action_attempt_id), restrict artifact_type to artifacts that are actually attempt-scoped for local runs (REPORT/OUTPUTS per the documented layout).
// CreateLocalDownloadLinkRequest specifies the request for the CreateLocalDownloadLink API.
message CreateLocalDownloadLinkRequest {
// ArtifactType is the type of artifact to download.
// +required
ArtifactType artifact_type = 1 [(buf.validate.field).enum = {
not_in: [0]
}];
flyteidl2/dataproxy/dataproxy_service.proto:279
- UploadMetadataRequest allows any pairing of (artifact_type, target). As written, clients can send ARTIFACT_TYPE_INPUTS with action_attempt_id (or OUTPUTS/REPORT with action_id), which contradicts the documented storage layout and makes server-side handling ambiguous. Add a protovalidate CEL constraint to enforce the correct target field based on artifact_type.
// ArtifactType is the type of artifact to upload. INPUTS targets an action; OUTPUTS and REPORT
// target an action attempt.
ArtifactType artifact_type = 1 [(buf.validate.field).enum = {
flyteidl2/dataproxy/dataproxy_service.proto:288
- content_length is documented as optional, but because it is a non-optional scalar in proto3, callers cannot distinguish "unset" from "0". If this field is truly optional, mark it
optionalto enable presence, and add a non-negative validation constraint (sizes should not be negative).
// ContentLength specifies the size of the content to be uploaded in bytes. Validated against
// the platform's maximum upload size.
// +optional
int64 content_length = 5;
- Files reviewed: 16/32 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Marks a local run and all its non-terminal actions ABORTED server-side. The platform cannot stop the local orchestrator, so this is a bookkeeping abort: subsequent reports against aborted actions are rejected, and aborting an already-terminal run is acknowledged as a no-op. Reuses RunService's AbortRunRequest/AbortRunResponse messages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VJa4n1pjJUBVQbjeWCZJyQ Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
There was a problem hiding this comment.
🟡 Not ready to approve
flyteidl2/workflow/local_run_service.proto includes an unused import (flyteidl2/task/common.proto) that is likely to fail buf/proto lint and should be removed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (13)
- gen/go/flyteidl2/dataproxy/dataproxy_service.pb.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxy_service.pb.validate.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxy_service_grpc.pb.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxyconnect/dataproxy_service.connect.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service.pb.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service.pb.validate.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service_grpc.pb.go: Generated file
- gen/go/flyteidl2/workflow/run_definition.pb.go: Generated file
- gen/go/flyteidl2/workflow/workflowconnect/local_run_service.connect.go: Generated file
- gen/go/flyteidl2/workflow/workflowconnect/mocks/mocks.go: Generated file
- gen/python/flyteidl2/dataproxy/dataproxy_service_pb2.py: Generated file
- gen/python/flyteidl2/workflow/local_run_service_pb2.py: Generated file
- gen/python/flyteidl2/workflow/run_definition_pb2.py: Generated file
Suppressed comments (1)
flyteidl2/workflow/local_run_service.proto:14
flyteidl2/task/common.protois imported but not referenced anywhere in this file. This can trigger buf/proto lint (unused import) and should be removed;flyteidl2/task/run.protoalready brings in any transitive task-common dependencies.
import "flyteidl2/task/common.proto";
- Files reviewed: 16/32 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Local runs no longer get dedicated dataproxy RPCs. Uploads use CreateUploadLocation and reads use GetActionData/CreateDownloadLink, routed like every other data operation: the client asks ClusterService.SelectCluster (new OPERATION_LOCAL_RUN_DATA) and the server answers with a dataplane endpoint when the org's clusters are directly reachable, or its own endpoint otherwise — in which case the control plane serves the artifacts from its own storage. Two small extensions to existing messages make this routable: - SelectClusterResponse.cluster names the selected cluster so reporters can stamp it on the action attempt; reads later route by the recorded attempt cluster (empty = control-plane storage). - GetActionDataRequest.run_source disambiguates resolution: run names are not unique across platform and local runs, so RUN_SOURCE_LOCAL resolves the action via LocalRunService. Removes UploadMetadata, GetLocalActionData, CreateLocalDownloadLink and their request messages, and the INPUTS/OUTPUTS artifact types that only they used. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VJa4n1pjJUBVQbjeWCZJyQ Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
There was a problem hiding this comment.
🟡 Not ready to approve
The IDL/comments and PR description contain mismatches (including “optional” semantics and referenced/missing DataProxy APIs), which should be resolved to avoid confusing or breaking downstream consumers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (14)
- gen/go/flyteidl2/cluster/payload.pb.go: Generated file
- gen/go/flyteidl2/cluster/payload.pb.validate.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxy_service.pb.go: Generated file
- gen/go/flyteidl2/dataproxy/dataproxy_service.pb.validate.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service.pb.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service.pb.validate.go: Generated file
- gen/go/flyteidl2/workflow/local_run_service_grpc.pb.go: Generated file
- gen/go/flyteidl2/workflow/run_definition.pb.go: Generated file
- gen/go/flyteidl2/workflow/workflowconnect/local_run_service.connect.go: Generated file
- gen/go/flyteidl2/workflow/workflowconnect/mocks/mocks.go: Generated file
- gen/python/flyteidl2/cluster/payload_pb2.py: Generated file
- gen/python/flyteidl2/dataproxy/dataproxy_service_pb2.py: Generated file
- gen/python/flyteidl2/workflow/local_run_service_pb2.py: Generated file
- gen/python/flyteidl2/workflow/run_definition_pb2.py: Generated file
Suppressed comments (3)
flyteidl2/dataproxy/dataproxy_service.proto:201
run_sourceis labeled as+optional, but it is not declaredoptionalin proto3, so most generated APIs cannot distinguish “unset” from the default enum value. In TS, the generated message type requiresrunSource, which can be a breaking type-level change for consumers. Consider making this a true proto3optionalfield (and regenerating code) or adjusting the comment to document the default behavior instead of implying presence semantics.
// Source of the run the action belongs to. Run names are not unique across platform and
// local runs, so the server uses this to resolve the action against the matching service
// (RUN_SOURCE_LOCAL resolves via LocalRunService; anything else keeps the platform
// behavior).
// +optional
workflow.RunSource run_source = 2;
flyteidl2/workflow/local_run_service.proto:98
- The comment references
DataProxyService.UploadMetadata, butDataProxyServicein this repo does not define anUploadMetadataRPC (it hasUploadInputs). This can confuse API consumers; please update the comment to point to the actual RPC that producesOffloadedInputData.
// Reference to the run's inputs, previously uploaded via DataProxyService.UploadMetadata.
optional common.OffloadedInputData offloaded_input_data = 5;
flyteidl2/dataproxy/dataproxy_service.proto:200
- The PR description states DataProxyService gains new local-run APIs (e.g. UploadMetadata/GetLocalActionData/CreateLocalDownloadLink and ArtifactType INPUTS/OUTPUTS), but this proto only adds
run_sourcetoGetActionDataRequestand does not define those RPCs or enum values. Please either update the PR description to match the actual changes or include the missing IDL changes in this PR.
This issue also appears on line 196 of the same file.
// Source of the run the action belongs to. Run names are not unique across platform and
// local runs, so the server uses this to resolve the action against the matching service
// (RUN_SOURCE_LOCAL resolves via LocalRunService; anything else keeps the platform
// behavior).
// +optional
- Files reviewed: 17/34 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Reworked per review discussion: the three dedicated local-run dataproxy RPCs are gone. Local-run data now flows through the existing APIs — |
| // The action's spec. Should be provided on the first report of an action; ignored afterwards. | ||
| oneof spec { | ||
| TaskAction task = 4; | ||
| TraceAction trace = 5; |
There was a problem hiding this comment.
how do we restrict condition action, we should test that it fails gracefully
flyteorg/flyte#7737 merged as 1d16a6781; repoint the temporary git pin from the feature branch to main and refresh the comments, which still referenced the DataProxyService.UploadMetadata RPC that PR deleted. The pin stays temporary — a released flyteidl2 is now unblocked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VJa4n1pjJUBVQbjeWCZJyQ Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Overview
Adds
flyteidl2.workflow.LocalRunService: clients that orchestrate actions outside the platform (e.g. on a user's machine) register a run and report action state, while the platform only records and serves it — no queueing, no dataplane.Design highlights:
GetRunDetails/WatchRunDetails,GetActionDetails/WatchActionDetails,ListRuns/WatchRuns,ListActions/WatchActions) reuses RunService's request/response messages verbatim, so existing clients and UIs work against local runs with only a service swap. The two new shapes areCreateLocalRunRequestandReportLocalActionsRequest(batch, idempotent on the event's(attempt, version, phase)).DataProxyServicegains the control-plane-direct data surface for local runs:UploadMetadata(signed PUT URLs for inputs.pb/outputs.pb/report.html),GetLocalActionData,CreateLocalDownloadLink, plusArtifactTypeINPUTS/OUTPUTSvalues.RunSource.RUN_SOURCE_LOCALmarks these runs.buf lint's message-reuse rules are ignored for the affected files (same precedent as cacheservice/project).
Test Plan
Codegen (
make buf-*+ mockery) committed for go/ts/python/rust; consumed and exercised end-to-end by the union cloud implementation (service + integration harness driving create → report → watch streams → reads).Rollout Plan
IDL + generated code only; no server behavior in this repo changes. Server implementation lands in the consuming repo.
Rollback Plan
Revert; no persisted state depends on this.
🤖 Generated with Claude Code