sdk-go icon indicating copy to clipboard operation
sdk-go copied to clipboard

Provide access to the workflow results from worker.WorkflowReplayer

Open kylelemons opened this issue 1 year ago • 0 comments

Is your feature request related to a problem? Please describe. The worker.WorkflowReplayer interface does not have the GetWorkflowResult method on it, even though the underlying WorkflowReplayer struct does.

I am using this to build a test helper to replay workflows and ensure that their results are the same each time, and so I currently need to to my own interface assertion to get access to this method:

type replayerWithResults interface {
	worker.WorkflowReplayer

	// GetWorkflowResult returns the result of a workflow execution.
	//
	// For replays, the workflowID will always be "ReplayId".
	//
	// https://pkg.go.dev/go.temporal.io/[email protected]/internal#WorkflowReplayer.GetWorkflowResult
	GetWorkflowResult(workflowID string, valuePtr any) error
}

Describe the solution you'd like It's a bit challenging to make this a completely backward compatible change, since this API is returning an interface rather than a concrete type.

Ideally, it would switch to returning the WorkflowReplayer struct directly from e.g. NewWorkflowReplayerWithOptions, so that new methods can be added backward-compatibly. However, I think that ship has sailed, so there are a few options:

  • Create a new API that returns the concrete type, e.g. NewWorkflowReplayerWithResults.
  • Expose the internal.WorkflowReplayer type via a type alias so that users can type assert the type directly, and promise that this is the type that will be returned from these constructors.
  • Create another interface (e.g. WorkflowReplayerWithResults) and document that it will be returned from these constructors.
  • Document that the returned type has additional methods that users can use in their own interfaces and what they are.

I think the first one is probably the optimal one from a user standpoint, but the second one is likely to be close enough, and it is future proof in a way that the final two are not.

kylelemons avatar Aug 27 '24 21:08 kylelemons