Add replay command #3617

lopert · 2024-03-28T15:29:17Z

WHY are these changes introduced?

Part of the LogStreaming Prototype project.

WHAT is this pull request doing?

adds a new command shopify app function replay, which:
- reads a folder for run logs
- presents the list of runs in a selector
- parses the input from the selected run
- forwards the run to function-runner

How to test your changes?

checkout this branch in your local CLI
export SHOPIFY_CLI_ENABLE_APP_LOG_POLLING=1
run app dev and run through checkout to trigger some logs, confirm they are being pulled.
run the following command from the CLI directory

pnpm shopify app function replay --path=<path-to-your-ext>

you might need to specify the export as well, ex:

pnpm shopify app function replay --path /Users/lopert/code/log-streaming/app/log-streamer/extensions/product-discount -e "run"

github-actions · 2024-04-28T03:34:13Z

This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action.
→ If there's no activity within a week, then a bot will automatically close this.
Thanks for helping to improve Shopify's dev tooling and experience.

lopert · 2024-04-29T14:33:06Z

Some activity to keep the PR open, will pick this up after ATC work this week.

github-actions · 2024-05-06T20:16:06Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	71.93% (+0.04% 🔼)	7278/10118
🟡	Branches	69.01% (-0.01% 🔻)	3572/5176
🟡	Functions	71.6% (+0.06% 🔼)	1946/2718
🟡	Lines	73.23% (+0.09% 🔼)	6854/9359

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🔴	`...` / replay.ts	0%	0%	0%	0%
🟢	`...` / replay.ts	100%	100%	100%	100%
🟢	`...` / replay.ts	100%	66.67%	100%	100%

Test suite run success

1688 tests passing in 785 suites.

Report generated by 🧪jest coverage report action from a5364d1

github-actions · 2024-05-10T18:14:24Z

We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

docs-shopify.dev/commands/app-function-replay.doc.ts

DuncanUszkay1 · 2024-05-10T18:22:13Z

packages/app/src/cli/commands/app/function/replay.ts

+import {joinPath} from '@shopify/cli-kit/node/path'
+
+export default class FunctionReplay extends Command {
+  static summary = 'Replays a function locally based on a FunctionRunEvent.'


We've moving away from Event and towards Log, we should do a sweep here

If we expose this term externally, do we need documentation on what it means?

packages/app/src/cli/services/function/replay.ts

DuncanUszkay1

Thoughts:

How can we merge this while still keeping it hidden? Not to prevent someone from finding out, but just to not mislead them into thinking that it is ready to use
Let's talk more about the overall spec for the CLI commands on Monday

It seems like the best path forward here is to park this, start finalizing the app dev command including writing the files, and circle back once that's done. We should also have our specs cleared up by then and in every CLI issue.

mssalemi · 2024-05-13T13:15:28Z

Related to yarn dev, and probably something we can discuss at meeting later, but how do we think these files should get saved from the yarn dev? (I guess we depend on each other on these PR's). Just thinking on how I should be saving these logs to a write. ✍️

Since there can be multiple function running, if we save all the runs the functions directory on invocation id, it might be confusing for developer to filter through that list. 🤔

docs-shopify.dev/commands/app-function-replay.doc.ts

packages/app/src/cli/commands/app/function/replay.ts

andrewhassan · 2024-05-13T19:53:07Z

packages/app/src/cli/commands/app/function/replay.ts

+import {joinPath} from '@shopify/cli-kit/node/path'
+
+export default class FunctionReplay extends Command {
+  static summary = 'Replays a function locally based on a FunctionRunEvent.'


If we expose this term externally, do we need documentation on what it means?

packages/app/src/cli/prompts/dev.ts

packages/app/src/cli/commands/app/function/replay.ts

packages/app/src/cli/services/function/replay.ts

DuncanUszkay1 · 2024-06-03T17:40:04Z

packages/app/src/cli/commands/app/function/replay.ts

+    export: Flags.string({
+      char: 'e',
+      hidden: false,
+      description: 'Name of the Web Assembly export to invoke.',
+      default: '_start',
+      env: 'SHOPIFY_FLAG_EXPORT',
+    }),


💭 Isn't the export being invoked defined by the run itself? E.G. a run using fetch couldn't be sensibly rerun using run. Perhaps that should be part of the payload so you never have to specify it here?

Probably not urgent for this PR, but would be an improvement for later.

🤔 It's one of the flags that we can invoke function runner with so that's how it ended up here.
The export is defined in the extension's shopify.extension.toml, so I think we could maybe improve this by just reading from there?

Yeah, I think as a future improvement, it'd be nice to read the export from the function config. I'm thinking of some logic like this:

If there's no export: default to _start because it's a legacy function

If there's a single export: default to that export (unless overridden with the -e flag)

If there's multiple exports: either let them select the export or fail saying one needs to be provided with the -e flag

I agree with @DuncanUszkay1 that this should be part of the payload, the input you are replaying would be specific to a certain target.

Will add that to the payload in my upcoming PR that also adds source/source namespace- we can probably do a follow-up CLI PR that starts to use all of those fields

packages/app/src/cli/prompts/dev.test.ts

packages/app/src/cli/services/function/replay.ts

andrewhassan

Overall looking pretty good! Just have some comments 😄

packages/app/src/cli/commands/app/function/replay.ts

andrewhassan · 2024-06-04T17:34:52Z

packages/app/src/cli/commands/app/function/replay.ts

+    export: Flags.string({
+      char: 'e',
+      hidden: false,
+      description: 'Name of the Web Assembly export to invoke.',
+      default: '_start',
+      env: 'SHOPIFY_FLAG_EXPORT',
+    }),


Yeah, I think as a future improvement, it'd be nice to read the export from the function config. I'm thinking of some logic like this:

If there's no export: default to _start because it's a legacy function

If there's a single export: default to that export (unless overridden with the -e flag)

If there's multiple exports: either let them select the export or fail saying one needs to be provided with the -e flag

packages/app/src/cli/commands/app/function/replay.ts

packages/app/src/cli/prompts/dev.ts

packages/app/src/cli/services/app-logs/write-app-logs.ts

packages/app/src/cli/services/function/replay.ts

packages/app/src/cli/services/function/replay.test.ts

packages/app/src/cli/services/function/replay.ts

DuncanUszkay1

LGTM aside from dealing with the 'no logs to replay' case explicitly

packages/app/src/cli/services/app-logs/write-app-logs.ts

packages/app/src/cli/services/function/replay.test.ts

packages/app/src/cli/services/function/replay.ts

github-actions · 2024-06-05T20:00:20Z

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

Some seemingly private modules might be re-exported through public modules.
If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/logs.d.ts

@@ -1,3 +1,3 @@
-export declare const getLogsDir: string;
+export declare const getLogsDir: () => string;
 export declare const createLogsDir: (path: string) => Promise<void>;
 export declare const writeLog: (path: string, logData: string) => Promise<void>;
\ No newline at end of file

lopert force-pushed the lopert.runs-selector branch 2 times, most recently from 7375b0e to 269565e Compare March 28, 2024 19:41

github-actions bot added the no-pr-activity label Apr 28, 2024

lopert removed the no-pr-activity label Apr 29, 2024

lopert force-pushed the lopert.runs-selector branch from 269565e to 384610e Compare May 6, 2024 20:12

lopert force-pushed the lopert.runs-selector branch 5 times, most recently from e1e31d7 to 011d2a3 Compare May 10, 2024 16:05

lopert requested review from mssalemi and DuncanUszkay1 May 10, 2024 17:08

lopert force-pushed the lopert.runs-selector branch from 011d2a3 to 8d5efce Compare May 10, 2024 17:23

lopert marked this pull request as ready for review May 10, 2024 18:14

DuncanUszkay1 reviewed May 10, 2024

View reviewed changes

docs-shopify.dev/commands/app-function-replay.doc.ts Outdated Show resolved Hide resolved

DuncanUszkay1 reviewed May 10, 2024

View reviewed changes

packages/app/src/cli/services/function/replay.ts Outdated Show resolved Hide resolved

DuncanUszkay1 reviewed May 10, 2024

View reviewed changes

packages/app/src/cli/services/function/replay.ts Outdated Show resolved Hide resolved

DuncanUszkay1 reviewed May 10, 2024

View reviewed changes

packages/app/src/cli/services/function/replay.ts Outdated Show resolved Hide resolved

DuncanUszkay1 reviewed May 10, 2024

View reviewed changes

packages/app/src/cli/services/function/replay.ts Outdated Show resolved Hide resolved

DuncanUszkay1 reviewed May 10, 2024

View reviewed changes

andrewhassan reviewed May 13, 2024

View reviewed changes

shauns added the #gsd:39776 label May 22, 2024

lopert force-pushed the lopert.runs-selector branch 2 times, most recently from edf7099 to 082bb63 Compare May 31, 2024 16:04

DuncanUszkay1 reviewed Jun 3, 2024

View reviewed changes

packages/app/src/cli/prompts/dev.test.ts Outdated Show resolved Hide resolved

DuncanUszkay1 reviewed Jun 3, 2024

View reviewed changes

packages/app/src/cli/services/function/replay.ts Outdated Show resolved Hide resolved

use exec's input arg to pass in payload input

2338558

andrewhassan reviewed Jun 4, 2024

View reviewed changes

lopert added 6 commits June 4, 2024 15:54

description updates

923c335

early return replay if envVar not set

59db2dc

move selectFunctionRun prompt

9c3421c

derive filename from date object in write

a103a8a

identifierFromFilename should always return string

d0380b4

docs

1c4ee7b

DuncanUszkay1 reviewed Jun 5, 2024

View reviewed changes

packages/app/src/cli/services/function/replay.test.ts Outdated Show resolved Hide resolved

DuncanUszkay1 reviewed Jun 5, 2024

View reviewed changes

packages/app/src/cli/services/function/replay.ts Show resolved Hide resolved

DuncanUszkay1 approved these changes Jun 5, 2024

View reviewed changes

limit log lookup to 100

dba2b8d

lopert force-pushed the lopert.runs-selector branch from 91b04be to dba2b8d Compare June 5, 2024 18:20

throw when no logs

8684e0b

andrewhassan approved these changes Jun 5, 2024

View reviewed changes

packages/app/src/cli/services/app-logs/write-app-logs.ts Show resolved Hide resolved

packages/app/src/cli/services/function/replay.test.ts Outdated Show resolved Hide resolved

packages/app/src/cli/services/function/replay.ts Outdated Show resolved Hide resolved

use AbortError

a5364d1

lopert force-pushed the lopert.runs-selector branch from 726eaf5 to a5364d1 Compare June 5, 2024 19:58

lopert added this pull request to the merge queue Jun 5, 2024

Merged via the queue into main with commit 7ab6f92 Jun 5, 2024

lopert deleted the lopert.runs-selector branch June 5, 2024 20:33

shopify-shipit bot temporarily deployed to nightly June 6, 2024 09:13 Inactive

shopify-shipit bot had a problem deploying to experimental June 6, 2024 11:02 Failure

shopify-shipit bot had a problem deploying to experimental June 6, 2024 11:05 Failure

shopify-shipit bot temporarily deployed to experimental June 6, 2024 11:26 Inactive

shopify-shipit bot temporarily deployed to production June 18, 2024 09:26 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add replay command #3617

Add replay command #3617

lopert commented Mar 28, 2024 •

edited

Loading

github-actions bot commented Apr 28, 2024

lopert commented Apr 29, 2024

github-actions bot commented May 6, 2024 •

edited

Loading

github-actions bot commented May 10, 2024

DuncanUszkay1 May 10, 2024

andrewhassan May 13, 2024

DuncanUszkay1 left a comment •

edited

Loading

mssalemi commented May 13, 2024

andrewhassan May 13, 2024

DuncanUszkay1 Jun 3, 2024

lopert Jun 3, 2024

andrewhassan Jun 4, 2024

nickwesselman Jun 5, 2024

DuncanUszkay1 Jun 5, 2024 •

edited

Loading

andrewhassan left a comment

andrewhassan Jun 4, 2024

DuncanUszkay1 left a comment

github-actions bot commented Jun 5, 2024

Add replay command #3617

Add replay command #3617

Conversation

lopert commented Mar 28, 2024 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

How to test your changes?

github-actions bot commented Apr 28, 2024

lopert commented Apr 29, 2024

github-actions bot commented May 6, 2024 • edited Loading

Coverage report

Test suite run success

github-actions bot commented May 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DuncanUszkay1 left a comment • edited Loading

Choose a reason for hiding this comment

mssalemi commented May 13, 2024

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

Choose a reason for hiding this comment

DuncanUszkay1 Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

andrewhassan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DuncanUszkay1 left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 5, 2024

Differences in type declarations

New type declarations

Existing type declarations

lopert commented Mar 28, 2024 •

edited

Loading

github-actions bot commented May 6, 2024 •

edited

Loading

DuncanUszkay1 left a comment •

edited

Loading

DuncanUszkay1 Jun 5, 2024 •

edited

Loading