temporalio / cli

Command-line interface for running Temporal Server and interacting with Workflows, Activities, Namespaces, and other parts of Temporal
https://docs.temporal.io/cli
MIT License
253 stars 39 forks source link

[Feature Request] reset should not require workflowid if runid is provided #710

Open tsurdilo opened 2 weeks ago

tsurdilo commented 2 weeks ago

https://github.com/temporalio/cli/blob/main/temporalcli/commands.workflow_reset.go#L33

currently reset requires workflow id and if not present seems to fall back to batch reset request is to also check presence of runid and if provided not to require workflow id, before falling to batch reset

cretz commented 2 weeks ago

Makes sense. To confirm we require workflow ID for all workflow-specific operations and also allow run ID if the user wants. It makes sense that if workflow ID is not present, we should fail if run ID is present.

josh-berry commented 1 week ago

@tsurdilo Can you give me an example command that's accepted but shouldn't be?

It looks like we already fail reset commands if workflow ID is provided but run ID isn't:

https://github.com/temporalio/cli/blob/dd3072a5dc1baa4d105b7ce2ea8b0225a4fb632c/temporalcli/commands.workflow_reset.go#L57

cretz commented 1 week ago

It looks like we already fail reset commands if workflow ID is provided but run ID isn't

I had understood the issue as us not properly failing if a run ID only is provided. But I may have misunderstood.

josh-berry commented 1 week ago

ah okay. Yeah, on second reading, I think from the title it's that we want to allow something like temporal workflow reset --run-id ... without also requiring --workflow-id. I don't know if this is even possible (I thought the run ID's uniqueness is scoped to a single workflow but I might be wrong).

cretz commented 1 week ago

While run ID is globally unique IIUC, I am not aware of any workflow-specific operation where we've ever accepted run ID without a workflow ID. I think we should always fail if a workflow ID is missing for a workflow-specific call (so if run ID is present here without workflow ID, we should fail IMO). However, if this is something unique to reset we need to support, we should first confirm only-run-ID even works on the server, then we can discuss if we even want this. I am not sure we do.

josh-berry commented 1 week ago

👍 Would like to hear what @tsurdilo had in mind for a use case / context about why this is needed. But if there's not a compelling reason then I agree with closing.