Conversation
4ccb9d7 to
14ea604
Compare
| // | ||
| // The per-request timeout matches the SDK's PollActivityResult implementation. | ||
| // Unlike the SDK, we retry at the application level on per-request timeout | ||
| // since we don't have the SDK's gRPC-level retry interceptor. |
There was a problem hiding this comment.
I don't like this. It doesn't need to block this PR, but it's an imperfect emulation of what the SDK is doing, which will diverge over time leading to more bugs. I am proposing that we introduce new poll methods in the Go SDK for Update and Activity that expose the raw proto responses to the CLI.
go.mod
Outdated
| module github.com/temporalio/cli | ||
|
|
||
| go 1.26.0 | ||
| go 1.25.5 |
There was a problem hiding this comment.
That was unintentional; the PR had incorrect dependency changes. Fixed now.
go.mod
Outdated
| go.temporal.io/sdk v1.39.1-0.20260205231726-1a609f101fd5 | ||
| go.temporal.io/sdk/contrib/envconfig v0.1.0 | ||
| go.temporal.io/server v1.30.0 | ||
| go.temporal.io/server v1.31.0-150.0 |
There was a problem hiding this comment.
Yes sorry the dependencies in this PR weren't finalized. I will use 151. But we'll have to relax the validator rules:
cli(standalone-activity-client) go run ./internal/cmd/validate-server-version/
Found server dependency: go.temporal.io/server@v1.31.0-151.2
✓ Version is a valid tagged version
Latest GitHub release: v1.29.3
Server version: v1.31.x
Latest release: v1.29.x
Max allowed: v1.30.x
Error: server dependency version v1.31.0-151.2 exceeds allowed range
Max allowed: v1.30.x (latest release + 1 minor)
Latest release: v1.29.3
exit status 1
go.mod
Outdated
| go.temporal.io/sdk v1.38.0 | ||
| github.com/temporalio/ui-server/v2 v2.45.0 | ||
| go.temporal.io/api v1.62.1 | ||
| go.temporal.io/sdk v1.39.1-0.20260205231726-1a609f101fd5 |
There was a problem hiding this comment.
Assuming that this will be bumped to a stable release before merging.
| short: r | ||
| description: | | ||
| Run ID. | ||
| If not set, targets the latest run. |
There was a problem hiding this comment.
Some commands the run ID is for the activity and for others for the workflow. We need to make sure we print out what the run ID refers to.
There was a problem hiding this comment.
Changed help text as follows:
--run-id string Run ID. For workflow Activities (when --workflow-id is provided), this is the Workflow Run ID. For Standalone Activities, this is the Activity Run ID.
| - name: activity-id | ||
| type: string | ||
| short: a | ||
| description: Activity ID. |
There was a problem hiding this comment.
Needs to be called out that this applies to either an ID of an activity within a workflow execution or a standalone activity execution ID.
There was a problem hiding this comment.
Changed to
description: |
Activity ID. This may be the ID of an Activity invoked by a Workflow, or of a Standalone
Activity.
internal/temporalcli/commands.yaml
Outdated
| required: true | ||
| - name: task-queue | ||
| type: string | ||
| description: Activity Task queue. |
There was a problem hiding this comment.
Why is Task capitalized and queue isn't?
| - name: heartbeat-timeout | ||
| type: duration | ||
| description: | | ||
| Maximum time between successful Worker heartbeats. |
There was a problem hiding this comment.
Also worth calling out that when expired, the current activity attempt fails.
| JSON values. | ||
| Can be passed multiple times. | ||
| See https://docs.temporal.io/visibility. | ||
| - name: headers |
There was a problem hiding this comment.
We don't expose this on workflow start or signal, we shouldn't for activity start either.
There was a problem hiding this comment.
Actually, workflow headers were added recently: #883
internal/temporalcli/commands.yaml
Outdated
| ``` | ||
|
|
||
| Activity code cannot see or respond to terminations. To | ||
| perform clean-up work, use `temporal activity cancel` instead. |
There was a problem hiding this comment.
This isn't accurate because if you activity is backing off it's the same as terminate. Cancelation only propagates to the activity if there's an active attempt and the activity is heartbeating.
There was a problem hiding this comment.
Thanks. Deleted second sentence so that it just says
Activity code cannot see or respond to terminations.
| - name: temporal activity start | ||
| summary: Start a new Standalone Activity (Experimental) | ||
| description: | | ||
| Start a new Standalone Activity. Outputs the Activity ID and |
There was a problem hiding this comment.
Why are the line lengths so short in this file?
There was a problem hiding this comment.
Not sure, but it's like that on main.
bergundy
left a comment
There was a problem hiding this comment.
Pre-approving it since the changes requested are minimal and I trust you to address before merging.
dandavison
left a comment
There was a problem hiding this comment.
Thanks for the review @bergundy. All comments should be addressed. I will merge when CI is passing.
go.mod
Outdated
| go.temporal.io/sdk v1.39.1-0.20260205231726-1a609f101fd5 | ||
| go.temporal.io/sdk/contrib/envconfig v0.1.0 | ||
| go.temporal.io/server v1.30.0 | ||
| go.temporal.io/server v1.31.0-150.0 |
There was a problem hiding this comment.
Yes sorry the dependencies in this PR weren't finalized. I will use 151. But we'll have to relax the validator rules:
cli(standalone-activity-client) go run ./internal/cmd/validate-server-version/
Found server dependency: go.temporal.io/server@v1.31.0-151.2
✓ Version is a valid tagged version
Latest GitHub release: v1.29.3
Server version: v1.31.x
Latest release: v1.29.x
Max allowed: v1.30.x
Error: server dependency version v1.31.0-151.2 exceeds allowed range
Max allowed: v1.30.x (latest release + 1 minor)
Latest release: v1.29.3
exit status 1
go.mod
Outdated
| module github.com/temporalio/cli | ||
|
|
||
| go 1.26.0 | ||
| go 1.25.5 |
There was a problem hiding this comment.
That was unintentional; the PR had incorrect dependency changes. Fixed now.
internal/temporalcli/commands.yaml
Outdated
| ``` | ||
|
|
||
| Activity code cannot see or respond to terminations. To | ||
| perform clean-up work, use `temporal activity cancel` instead. |
There was a problem hiding this comment.
Thanks. Deleted second sentence so that it just says
Activity code cannot see or respond to terminations.
| - name: activity-id | ||
| type: string | ||
| short: a | ||
| description: Activity ID. |
There was a problem hiding this comment.
Changed to
description: |
Activity ID. This may be the ID of an Activity invoked by a Workflow, or of a Standalone
Activity.| short: r | ||
| description: | | ||
| Run ID. | ||
| If not set, targets the latest run. |
There was a problem hiding this comment.
Changed help text as follows:
--run-id string Run ID. For workflow Activities (when --workflow-id is provided), this is the Workflow Run ID. For Standalone Activities, this is the Activity Run ID.
internal/temporalcli/commands.yaml
Outdated
| Maximum time an Activity task can stay in a task | ||
| queue before a Worker picks it up. |
| - name: heartbeat-timeout | ||
| type: duration | ||
| description: | | ||
| Maximum time between successful Worker heartbeats. |
| JSON values. | ||
| Can be passed multiple times. | ||
| See https://docs.temporal.io/visibility. | ||
| - name: headers |
There was a problem hiding this comment.
Actually, workflow headers were added recently: #883
| - name: temporal activity start | ||
| summary: Start a new Standalone Activity (Experimental) | ||
| description: | | ||
| Start a new Standalone Activity. Outputs the Activity ID and |
There was a problem hiding this comment.
Not sure, but it's like that on main.
4d7ad43 to
f2a40b6
Compare
The SDK (v1.39.0+) auto-enables TLS when API-key credentials are set, unless ConnectionOptions.TLSDisabled is true. The envconfig package (v0.1.0) predates this field and does not set it when TLS is explicitly disabled, so the SDK re-enables TLS despite the user's --tls=false. Bridge the gap by setting TLSDisabled in the CLI after profile.ToClientOptions() when the profile has TLS explicitly disabled. Bump cliext SDK dependency to v1.40.0 The cliext module was on go.temporal.io/sdk v1.32.1 while the root module uses v1.40.0. This caused the ConnectionOptions.TLSDisabled field (added in v1.39.0) to be unresolved when editing cliext.
f2a40b6 to
731c861
Compare
What was changed
Implement all Standalone Activity client commands.
Why?
Standalone Activity is a new product feature which the CLI will support.
Checklist
Yes. https://docs.temporal.io/cli/activity must be updated.