feature: roadrunner-temporal v6 — connect-rpc control plane#776
Open
rustatian wants to merge 13 commits into
Open
feature: roadrunner-temporal v6 — connect-rpc control plane#776rustatian wants to merge 13 commits into
rustatian wants to merge 13 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR ports roadrunner-temporal to the v6 line (module path + dependency set) and migrates the plugin control plane from Goridge net/rpc to Connect-RPC, updating the plugin’s exported RPC surface and the integration tests accordingly.
Changes:
- Migrate control RPC implementation to Connect-RPC handlers (
Plugin.RPC() (string, http.Handler)+ generatedTemporalServicehandler). - Port integration tests from Goridge
net/rpcclients to Connect-RPC clients (plus shared Connect test helpers). - Update module path to
/v6and refresh RoadRunner/Temporal-related dependencies to their v6/v2 lines.
Reviewed changes
Copilot reviewed 32 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/updates/updates_replay_test.go | Switch replay/download test calls from Goridge net/rpc to Connect-RPC client calls. |
| tests/tls/hp_tls_test.go | Port TLS suite’s activity/workflow name fetching to Connect-RPC. |
| tests/tls/disaster_tls_test.go | Replace informer/resetter Goridge calls with Connect-RPC helper calls. |
| tests/helpers/rpc.go | Add Connect-RPC helper clients for Temporal/Informer/Resetter services used by tests. |
| tests/helpers/helpers.go | Update test helper imports to /v6 module path + v6 dataconverter. |
| tests/go.mod | Update test module deps to api-go/v6, Connect, and v6 plugin module path replacement. |
| tests/go.sum | Dependency checksum updates for Connect + v6 RoadRunner APIs. |
| tests/general/rpc_test.go | Port RPC methods test to Connect-RPC calls and v6 API protos. |
| tests/general/plugin_status_test.go | Update import to /v6 module path. |
| tests/general/hp_test.go | Port activity/workflow name fetching to Connect-RPC. |
| tests/general/general_test.go | Replace Goridge informer calls with Connect-RPC helper calls. |
| tests/general/disaster_test.go | Replace informer/resetter Goridge calls with Connect-RPC helper calls. |
| status.go | Update imports for RoadRunner pool v2 + api-plugins v6 status types. |
| rpc.go | Implement Connect-RPC TemporalService handler methods and soft-status responses. |
| queue/queue.go | Update internal import path to /v6. |
| queue/queue_test.go | Update internal import path to /v6. |
| plugin.go | Change exported RPC() to return Connect handler + update pool/process imports for v6/v2 deps. |
| metrics.go | Update pool imports to v2. |
| internal/codec/proto/proto.go | Update temporal protocol import to api-go/v6 and pool payload import to v2. |
| internal.go | Update pool/static pool imports to v2 and module imports to /v6. |
| info.go | Update goridge frame import to v4 and pool payload import to v2. |
| config.go | Update pool import to v2. |
| api/interfaces.go | Update pool/payload/worker/process imports to v2 and module imports to /v6. |
| aggregatedpool/workflow.go | Update payload import to v2 and module imports to /v6. |
| aggregatedpool/workers.go | Update module imports to /v6. |
| aggregatedpool/workers_test.go | Update module imports to /v6. |
| aggregatedpool/local_activity.go | Update goridge frame import to v4, payload import to v2, module imports to /v6. |
| aggregatedpool/interceptor.go | Update module import to /v6. |
| aggregatedpool/handler.go | Update goridge frame import to v4, payload import to v2, module imports to /v6. |
| aggregatedpool/activity.go | Update goridge frame import to v4, payload import to v2, module imports to /v6. |
| go.mod | Change module path to /v6 and add/update v6/v2 dependencies including Connect. |
| go.sum | Dependency checksum updates for Connect + v6/v2 RoadRunner deps. |
| go.work | Bump workspace Go version patch. |
| go.work.sum | Workspace checksum updates including Connect. |
Comments suppressed due to low confidence (1)
rpc.go:131
- ReplayWorkflow starts its history fetch with context.Background(), which drops the Connect handler's cancellation/deadline. This can cause unnecessary work if the client disconnects or times out. Use the handler context as the parent for the timeout context (and avoid the unused
_ context.Contextparameter).
func (r *rpc) ReplayWorkflow(_ context.Context, req *connect.Request[protoApi.ReplayRequest]) (*connect.Response[protoApi.ReplayResponse], error) {
in := req.Msg
out := &protoApi.ReplayResponse{}
r.plugin.log.Debug("replay workflow request",
zap.String("run_id", in.GetWorkflowExecution().GetRunId()),
zap.String("workflow_id", in.GetWorkflowExecution().GetWorkflowId()),
zap.String("workflow_name", in.GetWorkflowType().GetName()))
if in.GetWorkflowExecution() == nil || in.GetWorkflowType() == nil ||
in.GetWorkflowExecution().GetRunId() == "" || in.GetWorkflowExecution().GetWorkflowId() == "" || in.GetWorkflowType().GetName() == "" {
out.Status = newStatus(codes.InvalidArgument, "run_id, workflow_id or workflow_name should not be empty")
r.plugin.log.Error("replay workflow request", zap.String("error", "run_id, workflow_id or workflow_name should not be empty"))
return connect.NewResponse(out), nil
}
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
iter := r.plugin.temporal.client.GetWorkflowHistory(ctx, in.GetWorkflowExecution().GetWorkflowId(), in.GetWorkflowExecution().GetRunId(), false, enums.HISTORY_EVENT_FILTER_TYPE_ALL_EVENT)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+61
to
65
| ctx, err := r.plugin.temporal.rrActivityDef.GetActivityContext(req.Msg.GetTaskToken()) | ||
| if err != nil { | ||
| r.plugin.mu.RUnlock() | ||
| return err | ||
| return nil, err | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v6 line of the plugin, tracking the RoadRunner v6 plugin set.
github.com/temporalio/roadrunner-temporal/v6net/rpcto Connect-RPC:RPC() (string, http.Handler)returns the generatedTemporalServicehandler; soft errors keep v5 parity by reporting throughcommon.v1.Statusin the responselog/slogto match the v6 plugin set (endure wires interfaces structurally; the Temporal SDK logger now goes throughlog.NewStructuredLogger)roadrunner-server/api/v4→api-go/v6+api-plugins/v6,pool→pool/v2,goridgedroppedtemporal server start-dev)/v6moduleKnown red until the PHP worker speaks Connect: the PHP→RR reverse RPC path (e.g.
temporal.RecordActivityHeartbeaton :6001) — the heartbeat tests ingeneral/tlsfail on it (now gracefully instead of panicking).Note:
api-go/v6is pinned to a pseudo-version until roadrunner-server/api#75 and roadrunner-server/api-go#21 merge and a new beta tag is cut.