From 36faee947905ce5eb7543370b18fd530201c2c86 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Sat, 24 Feb 2024 20:24:37 -0500 Subject: [PATCH 1/5] Edits --- internal/activity.go | 13 ++++++------- internal/workflow.go | 19 +++++++++---------- workflow/workflow.go | 2 +- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/internal/activity.go b/internal/activity.go index d5d52a0c3..ccd6f91d6 100644 --- a/internal/activity.go +++ b/internal/activity.go @@ -79,14 +79,14 @@ type ( } // ActivityOptions stores all activity-specific parameters that will be stored inside of a context. - // The current timeout resolution implementation is in seconds and uses math.Ceil(d.Seconds()) as the duration. But is - // subjected to change in the future. + // The current timeout resolution implementation is in seconds and uses math.Ceil(d.Seconds()) as the duration. But this is + // subject to change in the future. ActivityOptions struct { - // TaskQueue that the activity needs to be scheduled on. - // optional: The default task queue with the same name as the workflow task queue. + // TaskQueue that the activity will be scheduled on. + // optional: The default is a task queue with the same name as the workflow task queue. TaskQueue string - // ScheduleToCloseTimeout - Total time that a workflow is willing to wait for Activity to complete. + // ScheduleToCloseTimeout - Total time that the workflow will wait for the Activity to complete. // ScheduleToCloseTimeout limits the total time of an Activity's execution including retries // (use StartToCloseTimeout to limit the time of a single attempt). // The zero value of this uses default value. @@ -119,8 +119,7 @@ type ( // Optional: default false WaitForCancellation bool - // ActivityID - Business level activity ID, this is not needed for most of the cases if you have - // to specify this then talk to temporal team. This is something will be done in future. + // ActivityID - Business-level activity ID. This is not typically needed. // Optional: default empty string ActivityID string diff --git a/internal/workflow.go b/internal/workflow.go index 70783a36a..f8b7c2534 100644 --- a/internal/workflow.go +++ b/internal/workflow.go @@ -120,35 +120,34 @@ type ( // json.Unmarshal. ReceiveWithTimeout(ctx Context, timeout time.Duration, valuePtr interface{}) (ok, more bool) - // ReceiveAsync try to receive from Channel without blocking. If there is data available from the Channel, it - // assign the data to valuePtr and returns true. Otherwise, it returns false immediately. + // ReceiveAsync tries to receive from a Channel without blocking. If there is data available, it + // assigns the data to valuePtr and returns true. Otherwise, it returns false immediately. // // Note, values should not be reused for extraction here because merging on // top of existing values may result in unexpected behavior similar to // json.Unmarshal. ReceiveAsync(valuePtr interface{}) (ok bool) - // ReceiveAsyncWithMoreFlag is same as ReceiveAsync with extra return value more to indicate if there could be - // more value from the Channel. The more is false when Channel is closed. + // ReceiveAsyncWithMoreFlag is the same as ReceiveAsync but with an extra return value `more` that indicates + // whether the channel contains more data. `more` is false when the channel is closed. // - // Note, values should not be reused for extraction here because merging on - // top of existing values may result in unexpected behavior similar to - // json.Unmarshal. + // Note, values should not be reused for extraction here because merging on top of existing values may result in + // unexpected behavior similar to json.Unmarshal. ReceiveAsyncWithMoreFlag(valuePtr interface{}) (ok bool, more bool) // Len returns the number of buffered messages plus the number of blocked Send calls. Len() int } - // Channel must be used instead of native go channel by workflow code. + // Channel must be used by workflow code instead of native go channels. // Use workflow.NewChannel(ctx) method to create Channel instance. Channel interface { SendChannel ReceiveChannel } - // Selector must be used instead of native go select by workflow code. - // Create through workflow.NewSelector(ctx). + // Selector must be used by workflow code instead of native go `select`. + // Use workflow.NewSelector(ctx) to create a selector. Selector interface { // AddReceive registers a callback function to be called when a channel has a message to receive. // The callback is called when Select(ctx) is called. diff --git a/workflow/workflow.go b/workflow/workflow.go index 17e509799..1fe41aeef 100644 --- a/workflow/workflow.go +++ b/workflow/workflow.go @@ -274,7 +274,7 @@ func SignalExternalWorkflow(ctx Context, workflowID, runID, signalName string, a return internal.SignalExternalWorkflow(ctx, workflowID, runID, signalName, arg) } -// GetSignalChannel returns channel corresponding to the signal name. +// GetSignalChannel returns the channel corresponding to the signal name. func GetSignalChannel(ctx Context, signalName string) ReceiveChannel { return internal.GetSignalChannel(ctx, signalName) } From 2750b1bfed0bb9b399006d3bd939e098b545712c Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Thu, 11 Jul 2024 13:38:30 -0400 Subject: [PATCH 2/5] edits --- internal/internal_workflow.go | 12 ++++++------ internal/workflow.go | 9 +++++---- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/internal/internal_workflow.go b/internal/internal_workflow.go index cc6eb852d..1ae2364ea 100644 --- a/internal/internal_workflow.go +++ b/internal/internal_workflow.go @@ -112,8 +112,8 @@ type ( Close() // Destroys all coroutines without waiting for their completion StackTrace() string // Stack trace of all coroutines owned by the Dispatcher instance - // Create coroutine. To be called from within other coroutine. - // Used by the interceptors + // NewCoroutine creates a new coroutine. To be called from within another coroutine. + // Used by the interceptors. NewCoroutine(ctx Context, name string, highPriority bool, f func(ctx Context)) Context } @@ -990,13 +990,13 @@ func (c *channelImpl) assignValue(from interface{}, to interface{}) error { return err } -// initialYield called at the beginning of the coroutine execution -// stackDepth is the depth of top of the stack to omit when stack trace is generated +// initialYield is called at the beginning of coroutine execution. +// stackDepth is the depth of the top of the stack to omit when a stack trace is generated, // to hide frames internal to the framework. func (s *coroutineState) initialYield(stackDepth int, status string) { if s.blocked.Swap(true) { - panic("trying to block on coroutine which is already blocked, most likely a wrong Context is used to do blocking" + - " call (like Future.Get() or Channel.Receive()") + panic("trying to block on a coroutine which is already blocked: most likely an incorrect Context was used to do a blocking " + + "call, such as Future.Get() or Channel.Receive()") } keepBlocked := true for keepBlocked { diff --git a/internal/workflow.go b/internal/workflow.go index f8b7c2534..a01020c9f 100644 --- a/internal/workflow.go +++ b/internal/workflow.go @@ -531,16 +531,17 @@ func NewSemaphore(ctx Context, n int64) Semaphore { return &semaphoreImpl{size: n} } -// Go creates a new coroutine. It has similar semantic to goroutine in a context of the workflow. +// Go creates a new coroutine in workflow code. It has similar semantics to native goroutines, but these must not be +// used in workflow code. func Go(ctx Context, f func(ctx Context)) { assertNotInReadOnlyState(ctx) state := getState(ctx) state.dispatcher.interceptor.Go(ctx, "", f) } -// GoNamed creates a new coroutine with a given human readable name. -// It has similar semantic to goroutine in a context of the workflow. -// Name appears in stack traces that are blocked on this Channel. +// GoNamed creates a new coroutine in workflow code, with a given human-readable name. It has similar semantics to +// native goroutines, but these must not be used in workflow code. Name appears in stack traces that are blocked on this +// Channel. func GoNamed(ctx Context, name string, f func(ctx Context)) { assertNotInReadOnlyState(ctx) state := getState(ctx) From e39e8d299173ab9e61e21d0eb9994c08f6f6bf4d Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Sat, 16 Dec 2023 10:23:05 -0500 Subject: [PATCH 3/5] Edits --- client/client.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/client/client.go b/client/client.go index b65126a5b..a0593fc14 100644 --- a/client/client.go +++ b/client/client.go @@ -525,9 +525,9 @@ type ( // GetRunID() will always return "run ID 1" and Get(ctx context.Context, valuePtr interface{}) will return the result of second run. GetWorkflow(ctx context.Context, workflowID string, runID string) WorkflowRun - // SignalWorkflow sends a signals to a workflow in execution + // SignalWorkflow sends a signals to a running workflow. // - workflow ID of the workflow. - // - runID can be default(empty string). if empty string then it will pick the running execution of that workflow ID. + // - runID can be default(empty string). If empty string then it will pick the running execution of that workflow ID. // - signalName name to identify the signal. // The errors it can return: // - serviceerror.NotFound @@ -537,10 +537,10 @@ type ( // SignalWithStartWorkflow sends a signal to a running workflow. // If the workflow is not running or not found, it starts the workflow and then sends the signal in transaction. - // - workflowID, signalName, signalArg are same as SignalWorkflow's parameters - // - options, workflow, workflowArgs are same as StartWorkflow's parameters + // - workflowID, signalName, signalArg are the same as SignalWorkflow's parameters + // - options, workflow, workflowArgs are the same as StartWorkflow's parameters // - the workflowID parameter is used instead of options.ID. If the latter is present, it must match the workflowID. - // Note: options.WorkflowIDReusePolicy is default to AllowDuplicate in this API. + // Note: options.WorkflowIDReusePolicy defaults to AllowDuplicate in this API. // The errors it can return: // - serviceerror.NotFound // - serviceerror.InvalidArgument From 0c8c780fab5a86732b43b195198727cd5e562756 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Thu, 11 Jul 2024 14:06:24 -0400 Subject: [PATCH 4/5] Code review suggestions --- client/client.go | 2 +- internal/workflow.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client/client.go b/client/client.go index a0593fc14..65dfca4e9 100644 --- a/client/client.go +++ b/client/client.go @@ -527,7 +527,7 @@ type ( // SignalWorkflow sends a signals to a running workflow. // - workflow ID of the workflow. - // - runID can be default(empty string). If empty string then it will pick the running execution of that workflow ID. + // - runID can be default(empty string). If set to empty string, then it will pick the running execution of that workflow ID. // - signalName name to identify the signal. // The errors it can return: // - serviceerror.NotFound diff --git a/internal/workflow.go b/internal/workflow.go index a01020c9f..4d132f650 100644 --- a/internal/workflow.go +++ b/internal/workflow.go @@ -128,8 +128,8 @@ type ( // json.Unmarshal. ReceiveAsync(valuePtr interface{}) (ok bool) - // ReceiveAsyncWithMoreFlag is the same as ReceiveAsync but with an extra return value `more` that indicates - // whether the channel contains more data. `more` is false when the channel is closed. + // ReceiveAsyncWithMoreFlag is the same as ReceiveAsync but with an extra return value more that indicates + // whether the channel contains more data. more is false when the channel is closed. // // Note, values should not be reused for extraction here because merging on top of existing values may result in // unexpected behavior similar to json.Unmarshal. @@ -146,7 +146,7 @@ type ( ReceiveChannel } - // Selector must be used by workflow code instead of native go `select`. + // Selector must be used by workflow code instead of native go select. // Use workflow.NewSelector(ctx) to create a selector. Selector interface { // AddReceive registers a callback function to be called when a channel has a message to receive. @@ -531,7 +531,7 @@ func NewSemaphore(ctx Context, n int64) Semaphore { return &semaphoreImpl{size: n} } -// Go creates a new coroutine in workflow code. It has similar semantics to native goroutines, but these must not be +// Go creates a new coroutine in workflow code. It has similar semantics to native goroutines, which must not be // used in workflow code. func Go(ctx Context, f func(ctx Context)) { assertNotInReadOnlyState(ctx) @@ -540,7 +540,7 @@ func Go(ctx Context, f func(ctx Context)) { } // GoNamed creates a new coroutine in workflow code, with a given human-readable name. It has similar semantics to -// native goroutines, but these must not be used in workflow code. Name appears in stack traces that are blocked on this +// native goroutines, which must not be used in workflow code. name appears in stack traces that are blocked on this // Channel. func GoNamed(ctx Context, name string, f func(ctx Context)) { assertNotInReadOnlyState(ctx) From 27f0eafe20bf3768fe441c1ec1ae6c324533904f Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Mon, 15 Jul 2024 10:47:18 -0400 Subject: [PATCH 5/5] Edit comments --- test/integration_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration_test.go b/test/integration_test.go index d1ae6ba71..91974f1a2 100644 --- a/test/integration_test.go +++ b/test/integration_test.go @@ -1458,7 +1458,7 @@ func (ts *IntegrationTestSuite) TestUpdateInfo() { run, err := ts.client.ExecuteWorkflow(ctx, ts.startWorkflowOptions("test-update-info"), ts.workflows.UpdateInfoWorkflow) ts.Nil(err) - // Send an update request with a know update ID + // Send an update request with a known update ID handler, err := ts.client.UpdateWorkflow(ctx, client.UpdateWorkflowOptions{ UpdateID: "testID", WorkflowID: run.GetID(), @@ -1467,7 +1467,7 @@ func (ts *IntegrationTestSuite) TestUpdateInfo() { WaitForStage: client.WorkflowUpdateStageCompleted, }) ts.NoError(err) - // Verify the upate handler can access the update info and return the updateID + // Verify the update handler can access the update info and return the updateID var result string ts.NoError(handler.Get(ctx, &result)) ts.Equal("testID", result)