Skip to content

add Namespace method to Client interface #1898

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bentveljanzx
Copy link
Contributor

@bentveljanzx bentveljanzx commented Mar 31, 2025

What was changed

Added Namespace() string method to Client interface and its various implementors.

Why?

  • Primary motivation: I want to be able to pass a Temporal client into a function that retrieves the workflow history of a workflow.
    ie.
func ExtractUnhandledFailure(ctx context.Context, temporalClient client.Client, workflowId string, innerErr error) error {
	// ... -- more code above --- ...

	// NOTE(jae): 2025-03-21
	// Query in reverse and just get last event. With local testing the difference in performance is
	// - ~16 milliseconds to iterate over each history event for ~200 events
	// - ~1 millisecond to get last event for ~200 events
	resp, err := temporalClient.WorkflowService().GetWorkflowExecutionHistoryReverse(ctx, &workflowservice.GetWorkflowExecutionHistoryReverseRequest{
		// I want the ability to pass the Namespace() parameter into the WorkflowService() call
		Namespace:       temporalClient.Namespace(),
		MaximumPageSize: 1,
		Execution: &common.WorkflowExecution{
			WorkflowId: workflowId,
			RunId:      "",
		},
	})

	// ... -- more code below --- ...
}
  • Secondary motivation: We have multiple of our own Temporal struct wrappers that hold the "namespace" value. It would be more convenient if we could just retrieve it from the interface directly.

Checklist

  1. Closes: N/A

  2. How was this tested:

  • Added a unit test and asserted that it returns the default namespace
  1. Any docs updates needed? N/A

@bentveljanzx bentveljanzx requested a review from a team as a code owner March 31, 2025 23:32
@bentveljanzx bentveljanzx changed the title add namespace method to temporal client interface add Namespace method to Client interface Mar 31, 2025
@cretz
Copy link
Member

cretz commented Apr 1, 2025

Primary motivation: I want to be able to pass a Temporal client into a function that retrieves the workflow history of a workflow.

If you are able to pass workflow ID here, why can you not also pass namespace? I am not sure we need to mutate the interface for everyone in this way. If we did want to do this however, we'd probably consider returning all of the options used to create the client, not just one of them (the next person may request Address() string, etc). But usually we'd recommend you pass the data around you need for your methods instead of expecting to extract it from the client. If you needed type MyClient struct { client.Client; Namespace string } or whatever for your needs, that would be fine.

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.

2 participants