worker deployment update-version-compute-config#1001
Conversation
afab676 to
f6fdb9b
Compare
f6601f4 to
bdd4c48
Compare
|
@chaptersix I don't believe the test failure has anything to do with this particular PR. |
|
A couple things worth reconsidering here: Command naming -- The CLI uses Implicit delete behavior -- Currently if |
@chaptersix all great points. I will rework the patch! |
bdd4c48 to
6bedccf
Compare
|
@chaptersix sorry for the delay and thank you for your patience. I've updated the code per your review recommendations. |
| RequestId: requestID, | ||
| } | ||
|
|
||
| if c.Remove { |
There was a problem hiding this comment.
Though AwsLambdaFunctionArn, AwsLambdaAssumeRoleArn, AwsLambdaAssumeRoleExternalId are harmlessly ignored when --remove is passed in, there's precedence in this repo (i.e. see TemporalOperatorNexusEndpointCreateCommand) to guard against non-supported permutations. Maybe a guard like this in the top of the method:
if c.Remove && (c.AwsLambdaFunctionArn != "" || c.AwsLambdaAssumeRoleArn != "" || c.AwsLambdaAssumeRoleExternalId != "") {
return fmt.Errorf("--remove cannot be combined with --aws-lambda-function-arn, --aws-lambda-assume-role-arn, or --aws-lambda-assume-role-external-id")
}
|
|
||
| // awsLambdaProviderDetailsPayload returns the encoded Payload representing AWS | ||
| // Lambda compute provider details. | ||
| func (c *TemporalWorkerDeploymentUpdateVersionComputeConfigCommand) awsLambdaProviderDetailsPayload() (*commonpb.Payload, error) { |
There was a problem hiding this comment.
I think this looks functionally identical to TemporalWorkerDeploymentCreateVersionCommand.awsLambdaProviderDetailsPayload? Maybe refactor into a helper func
| if c.AwsLambdaAssumeRoleExternalId != "" { | ||
| providerDetails["role_external_id"] = c.AwsLambdaAssumeRoleExternalId | ||
| } | ||
| err := validateAWSLambdaProviderDetails(providerDetails) |
There was a problem hiding this comment.
Seems this will reject if all three AWS Lambda flags are not supplied together. Maybe worth it to specify in help all 3 must be supplied together
There was a problem hiding this comment.
The help strings for those three CLI options states that they are required when --aws-lambda-function-arn is specific
8fe9ef9 to
85f8a18
Compare
Adds implementation of the `UpdateWorkerDeploymentVersionComputeConfig` gRPC API call with a `temporal worker deployment replace-version-compute-config` CLI command. I used `replace` instead of `update` to be more explicit about the replacement semantics used by the command. Signed-off-by: Jay Pipes <jay.pipes@temporal.io>
Changes the `temporal worker deployment replace-version-compute-config` CLI command to `temporal worker deployment update-version-compute-config` to match other CLI commands. Adds an explicit `--remove` CLI flag to remove the compute config on the version. Signed-off-by: Jay Pipes <jay.pipes@temporal.io>
85f8a18 to
08a9b95
Compare
|
@fretz12 thanks very much for your code review and sorry for the long delay in pushing another revision. I've addressed your comments. thanks! |
…#1104) ## Summary Backports server-independent changes from `main` into `release/1.8.x`. Every commit here was verified to build and pass codegen against the release line's current dependency pins (`server v1.31.0`, `sdk v1.41.1`, `api v1.62.8`) — nothing pulls in the newer server/SDK/API that landed on `main` via #1017. Scope agreed as: bug fixes + CLI changes + CI/tooling. Dependency bumps (#1040, #1052, #1063) and server-version-dependent features are intentionally excluded. ## Included (22 commits, cherry-picked with `-x`) **CLI / bug fixes** - #1006 Fix help with value flags (addresses #1003 — `--help` with value flags like `--address 123` surfaced `pflag: help requested` as an error) - #1012 skip CountWorkflow in batch operations when `--yes` is set - #1016 Sort output of listing search attributes - #1029 Workflow delete now prompts for confirmation - #1033 Add persistence info to start-dev banner - #1047 Add `temporal schedule list-matching-times` command - #1056 Fix task-queue config set help: use real fairness weight flag names - #1059 Prefix dev server cluster ID with `dev-server-` - #1089 fix: tls is not added for profiles without tls - #1099 Clarify activity pause timeout behavior - #941 auto-generate deprecation warnings from YAML config **Tests** - #1005 Fix flakey test by disabling EC2 metadata lookup - #1020 Remove `time.Sleep()` in commands.taskqueue_test.go **CI / tooling** - #1015 pin alpine docker image to 3.23.4 - #1024 remediate missing-dependency-cooldown - #1034 Bump actions/upload-artifact from 4 to 7 - #1044 improve dependabot config - #1045 add PR template - #1054 pin and bump GitHub Actions to latest versions - #1057 use allow instead of ignore for dependency-type in dependabot config - #1080 Bump the github-actions group with 2 updates ## Excluded (rely on the new server/SDK version) #1017 (server bump v1.31.0 -> v1.32.0-157.0), #1046, #1087, #1001, #1091, #1084 ## Verification - `go build ./...` passes - all test packages compile - `make gen` reports no codegen drift - server/sdk/api pins unchanged from `release/1.8.x` ## CI endpoint fix (added) Also backports the API-key CI test endpoint change from #1087 (`us-east-1` -> `ca-central-1`) as a standalone CI-only commit. This resolves the `Request unauthorized` failure in the "Test cloud API key" steps on `release/1.8.x`. The rest of #1087 (Nexus Operation command code) is excluded as it depends on the new server version. --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Sai Asish Y <say.apm35@gmail.com> Co-authored-by: Kevin Woo <3469532+kevinawoo@users.noreply.github.com> Co-authored-by: Rodrigo Zhou <rodrigo.zhou@temporal.io> Co-authored-by: Stephan Behnke <stephanos@users.noreply.github.com> Co-authored-by: Jiechen Zhong <jiechen.zhong@temporal.io> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Kent Gruber <kent.gruber@temporal.io> Co-authored-by: picatz <14850816+picatz@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: John Votta <jpvotta@gmail.com> Co-authored-by: Nasit Sarwar Sony <nasitsony96@gmail.com> Co-authored-by: hussam-salah <156124396+hussam-salah@users.noreply.github.com> Co-authored-by: Sai Asish Y <say.apm35@gmail.com> Co-authored-by: Bitalizer <23104115+bitalizer@users.noreply.github.com> Co-authored-by: Sean Kane <spkane31@gmail.com> Co-authored-by: Jessica Laughlin <JLDLaughlin@users.noreply.github.com>
Adds implementation of the
UpdateWorkerDeploymentVersionComputeConfiggRPC API call with atemporal worker deployment update-version-compute-configCLI command. There is a--removeCLI flag that allows the user to explicitly delete the compute config from a Worker Deployment Version.