Make workflow show output json usable for replay #110

feedmeapples · 2023-02-11T07:04:41Z

What was changed

Made workflow show --output json produce replayable history

Why?

Allows exporting replayable history with syntax

workflow show --output json -w workflow_id > ~/history.json

Checklist

Closes
How was this tested:

E2E test here #171

Any docs updates needed?

bergundy · 2023-02-23T00:22:27Z

common/defs-flags.go

-	FlagInputSTQDefinition = "Optional Query input, in JSON format.\nFor multiple parameters, concatenate them and separate by space."
-	FlagInputFileSTQDefinition = "Passes optional Query input from a JSON file.\nIf there are multiple JSON, concatenate them and separate by space or newline.\n" + "Input from the command line will overwrite file input."
+	FlagInputSTQDefinition             = "Optional Query input, in JSON format.\nFor multiple parameters, concatenate them and separate by space."
+	FlagInputFileSTQDefinition         = "Passes optional Query input from a JSON file.\nIf there are multiple JSON, concatenate them and separate by space or newline.\n" + "Input from the command line will overwrite file input."


nit: why aren't we using multiline strings here for better readability?

I'll change in a new PR

cc @DJSanti

workflow/workflow_commands.go

bergundy · 2023-02-23T00:23:43Z

workflow/workflow_commands.go

-	var lastEvent historypb.HistoryEvent // used for print result of this run
+	var lastEvent historypb.HistoryEvent


Why remove the comment?

if lastEvent usage changes later in code then the comment needs to be updated as well. Figured it's better to give a good name to the variable than commenting it (i think the name is already good, or another option executionResult )

changed to wfResult, should clarify the variable better and not need the comment?

bergundy · 2023-02-23T00:26:22Z

workflow/workflow_commands.go

-		hIter := sdkClient.GetWorkflowHistory(tcCtx, wid, rid, watch, enumspb.HISTORY_EVENT_FILTER_TYPE_ALL_EVENT)
-		iter := &historyIterator{iter: hIter, maxFieldLength: maxFieldLength, lastEvent: &lastEvent}
-		err = output.PrintIterator(c, iter, po)
+		iter := sdkClient.GetWorkflowHistory(tcCtx, wid, rid, watch, enumspb.HISTORY_EVENT_FILTER_TYPE_ALL_EVENT)


Watch and JSON aren't compatible, can we ensure they're not both set?

bergundy · 2023-02-23T00:27:49Z

workflow/workflow_commands.go

+	history := &historypb.History{}
+	history.Events = events
+
+	common.PrettyPrintJSONObject(history)


Just verifying that this will not generate pretty output when stdout is not a TTY.

bergundy

This requires tests.

bergundy

I mostly want to see a test for this but otherwise LGTM.

feedmeapples · 2023-03-25T05:22:25Z

I mostly want to see a test for this but otherwise LGTM.

added a test here #171 separately as it requires temporaltest to exist in the server repo (and to not block the current PR). One more #170

bergundy · 2023-03-25T20:00:10Z

I’d rather use the cli for the test server so we utilize it as much as possible

feedmeapples · 2023-03-28T01:59:25Z

I’d rather use the cli for the test server so we utilize it as much as possible

i'm split between these options:

Ideally i would like integration tests to directly cover CLI dev server, the commands and options. To test one thing at a time instead of implicitly proxy testing CLI server in unrelated tests. I see your point though

update: changing to SDK based dev server

workflow/workflow_commands.go

alexshtin · 2023-03-28T23:10:57Z

common/util.go

 	if err != nil {
-		fmt.Printf("Error when try to print pretty: %v", err)
-		fmt.Println(o)
+		fmt.Fprintf(w, "Error when try to print pretty: %v", err)


All fmt.Printf should be fmt.Fprintf. No?

yes! i'll send separately to not mix

Co-authored-by: Alex Shtin <alex@shtin.com>

feedmeapples marked this pull request as draft February 11, 2023 07:04

feedmeapples force-pushed the export-history branch from 319a726 to b385f4c Compare February 11, 2023 07:06

feedmeapples force-pushed the export-history branch from b385f4c to 3b24b0c Compare February 21, 2023 19:26

feedmeapples marked this pull request as ready for review February 23, 2023 00:10

feedmeapples force-pushed the export-history branch from 3b24b0c to 15a7c6b Compare February 23, 2023 00:10

feedmeapples changed the title ~~Support exporting history for replay~~ Make workflow show output json good for replay Feb 23, 2023

feedmeapples requested review from bergundy and robholland February 23, 2023 00:10

bergundy reviewed Feb 23, 2023

View reviewed changes

workflow/workflow_commands.go Show resolved Hide resolved

bergundy reviewed Feb 23, 2023

View reviewed changes

feedmeapples mentioned this pull request Feb 23, 2023

Make iterator printer generic temporalio/tctl-kit#39

Merged

feedmeapples force-pushed the export-history branch 2 times, most recently from 97ce5b8 to c00c2f4 Compare March 23, 2023 22:14

bergundy changed the title ~~Make workflow show output json good for replay~~ Make workflow show output json usable for replay Mar 24, 2023

bergundy reviewed Mar 24, 2023

View reviewed changes

feedmeapples requested a review from bergundy March 25, 2023 05:23

feedmeapples requested a review from alexshtin March 28, 2023 15:52

feedmeapples added 4 commits March 28, 2023 14:12

Make workflow show output json good for replay

69fdff7

Use urfave context for printing pretty json

bc1b7db

Address comment

bed8307

.

92652d1

feedmeapples force-pushed the export-history branch from 49d0693 to 92652d1 Compare March 28, 2023 18:13

alexshtin approved these changes Mar 28, 2023

View reviewed changes

Update workflow/workflow_commands.go

fc71dfa

Co-authored-by: Alex Shtin <alex@shtin.com>

feedmeapples merged commit 48b50b2 into main Mar 29, 2023

feedmeapples deleted the export-history branch March 29, 2023 02:39

tomwheeler mentioned this pull request May 21, 2024

Output for temporal workflow show is base-64 encoded, unlike tctl #573

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make workflow show output json usable for replay #110

Make workflow show output json usable for replay #110

feedmeapples commented Feb 11, 2023 •

edited

Loading

bergundy Feb 23, 2023

feedmeapples Feb 23, 2023

bergundy Feb 23, 2023

feedmeapples Feb 23, 2023 •

edited

Loading

feedmeapples Mar 22, 2023

bergundy Feb 23, 2023

bergundy Feb 23, 2023

bergundy left a comment

bergundy left a comment

feedmeapples commented Mar 25, 2023 •

edited

Loading

bergundy commented Mar 25, 2023

feedmeapples commented Mar 28, 2023 •

edited

Loading

alexshtin Mar 28, 2023

feedmeapples Mar 29, 2023

		var lastEvent historypb.HistoryEvent // used for print result of this run
		var lastEvent historypb.HistoryEvent

Make workflow show output json usable for replay #110

Make workflow show output json usable for replay #110

Conversation

feedmeapples commented Feb 11, 2023 • edited Loading

What was changed

Why?

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feedmeapples Feb 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bergundy left a comment

Choose a reason for hiding this comment

bergundy left a comment

Choose a reason for hiding this comment

feedmeapples commented Mar 25, 2023 • edited Loading

bergundy commented Mar 25, 2023

feedmeapples commented Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feedmeapples commented Feb 11, 2023 •

edited

Loading

feedmeapples Feb 23, 2023 •

edited

Loading

feedmeapples commented Mar 25, 2023 •

edited

Loading

feedmeapples commented Mar 28, 2023 •

edited

Loading