Skip to content
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

Support extracting workflow and activity options from context #388

Merged
merged 3 commits into from
Mar 26, 2021

Conversation

jlegrone
Copy link
Contributor

Description, Motivation and Context

What was changed

This PR adds three new public functions to the workflow package:

  • GetChildWorkflowOptions
  • GetActivityOptions
  • GetLocalActivityOptions

Motivation & Context

In interceptors and workflow/activity wrapper functions, it was previously not possible to set default option values that could be overridden by the parent context.

By extracting the existing workflow or activity options from the workflow context, we can now conditionally set defaults only when a value has not already been specified:

type FooRequest struct {
	Bar string
}
type FooResponse struct {
	Baz string
}

// Helper function to be used by worker implementations.
func ExecuteChildWorkflowFoo(ctx workflow.Context, req FooRequest) (FooResponse, error) {
	// Set default options
	opts := workflow.GetChildWorkflowOptions(ctx)
	if opts.TaskQueue == "" {
		opts.TaskQueue = "foo"
	}
	if opts.WorkflowRunTimeout == 0 {
		opts.WorkflowRunTimeout = time.Minute
	}
	ctx = workflow.WithChildOptions(ctx, opts)

	// Execute workflow and return a result
	var resp FooResponse
	err := workflow.ExecuteChildWorkflow(ctx, "Foo", req).Get(ctx, &resp)
	return resp, err
}

Closes issue: N/A

How has this been tested?

The changeset should be covered by unit tests.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@CLAassistant
Copy link

CLAassistant commented Mar 24, 2021

CLA assistant check
All committers have signed the CLA.

@jlegrone jlegrone force-pushed the jlegrone/get-workflow-opts branch from a0e97b9 to 17259f9 Compare March 24, 2021 13:19
Comment on lines +106 to +109
// assertNonZero checks that every top level value, struct field, and item in a slice is a non-zero value.
func assertNonZero(t *testing.T, i interface{}) {
_assertNonZero(t, i, reflect.ValueOf(i).Type().Name())
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies if this is overwrought; I think it's nice to have peace of mind that tests would break if new fields are added but not accounted for in tests or conversion functions, but it's not essential to this changeset.

Copy link
Member

@mfateev mfateev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks for the contribution!

@mfateev
Copy link
Member

mfateev commented Mar 26, 2021

To merge the license header has to be added:




go run ./internal/cmd/tools/copyright/licensegen.go --verifyOnly
  | copyright header check failed, err=internal/workflow_test.go missing license header
  | exit status 255

Generated via 'go run ./internal/cmd/tools/copyright/licensegen.go'
@jlegrone jlegrone force-pushed the jlegrone/get-workflow-opts branch from 92d0f8b to 64bc3a5 Compare March 26, 2021 15:10
@mfateev mfateev merged commit 83d640e into temporalio:master Mar 26, 2021
@jlegrone jlegrone deleted the jlegrone/get-workflow-opts branch March 26, 2021 21:13
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.

3 participants