Implement `versions list` + `versions view` commands #5208

RamIdeas · 2024-03-08T14:40:06Z

What this PR solves / how to test

This PR implements the versions list and versions view <version-id> commands. They are very similar so a single PR seemed fine.

Author has addressed the following

Tests
- TODO (before merge)
- Included
- Not necessary because:
Changeset (Changeset guidelines)
- TODO (before merge)
- Included
- Not necessary because:
Public documentation
- TODO (before merge)
- Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/...
- Not necessary because: currently internal and experimental

changeset-bot · 2024-03-08T14:40:09Z

🦋 Changeset detected

Latest commit: efd3514

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
wrangler	Minor
@cloudflare/vitest-pool-workers	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2024-03-08T19:39:54Z

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8262522410/npm-package-wrangler-5208

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/5208/npm-package-wrangler-5208

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8262522410/npm-package-wrangler-5208 dev path/to/script.js

Additional artifacts:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8262522410/npm-package-create-cloudflare-5208 --no-auto-update

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8262522410/npm-package-cloudflare-kv-asset-handler-5208

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8262522410/npm-package-miniflare-5208

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8262522410/npm-package-cloudflare-pages-shared-5208

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8262522410/npm-package-cloudflare-vitest-pool-workers-5208

Note that these links will no longer work once the GitHub Actions artifact expires.

wrangler@3.33.0 includes the following runtime dependencies:

Package	Constraint	Resolved
`miniflare`	workspace:*	3.20240304.1
`workerd`	1.20240304.0	1.20240304.0
`workerd --version`	1.20240304.0	2024-03-04

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

codecov · 2024-03-08T19:43:20Z

Codecov Report

Attention: Patch coverage is 86.00000% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 70.98%. Comparing base (29e8151) to head (efd3514).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5208      +/-   ##
==========================================
+ Coverage   70.91%   70.98%   +0.07%     
==========================================
  Files         301      304       +3     
  Lines       15831    15927      +96     
  Branches     4053     4080      +27     
==========================================
+ Hits        11226    11306      +80     
- Misses       4605     4621      +16

Files	Coverage Δ
packages/wrangler/src/index.ts	`90.38% <100.00%> (-0.07%)`	⬇️
packages/wrangler/src/metrics/send-event.ts	`100.00% <ø> (ø)`
...kages/wrangler/src/utils/render-labelled-values.ts	`100.00% <100.00%> (ø)`
packages/wrangler/src/versions/index.ts	`38.46% <100.00%> (+11.18%)`	⬆️
packages/wrangler/src/versions/view.ts	`100.00% <100.00%> (ø)`
packages/wrangler/src/versions/list.ts	`75.86% <75.86%> (ø)`

... and 2 files with indirect coverage changes

mrbbot

Added some comments, but all non-blocking. Looks good! ✅ Assuming no changeset as this is still experimental, or are you planning to add one before merging?

mrbbot · 2024-03-12T17:38:14Z

packages/wrangler/src/__tests__/versions/versions.view.test.ts

+				`[APIError: A request to the Cloudflare API (/accounts/some-account-id/workers/scripts/test-name/versions/ffffffff-ffff-ffff-ffff-ffffffffffff) failed.]`
+			);
+
+			expect(normalizeOutput(std.out)).toMatchInlineSnapshot(`


Should this not be logged to stderr?

Interesting. This is just the fetchResult call rejecting. Which should bubble up to the main() try..catch error handling. Does wrangler log errors to stdout??

mrbbot · 2024-03-12T17:38:57Z

packages/wrangler/src/utils/render-labelled-values.ts

@@ -0,0 +1,22 @@
+import { logger } from "../logger";
+
+export default function renderLabelledValues(


A comment with example output from this function might be nice.

mrbbot · 2024-03-12T17:40:19Z

packages/wrangler/src/versions/list.ts

+type UUID = string;
+type VersionId = UUID;
+type ApiVersion = {
+	id: VersionId;
+	number: number;
+	metadata: {
+		created_on: string;
+		modified_on: string;
+		source: "api" | string;
+		author_id: string;
+		author_email: string;
+	};
+	annotations?: {
+		"workers/triggered_by"?: "upload" | string;
+		"workers/message"?: string;
+		"workers/tag"?: string;
+	};
+	// other properties not typed as not used
+};


Do these types get extracted into some common file in a later PR? They seem to be duplicated.

mrbbot · 2024-03-12T17:41:43Z

packages/wrangler/src/versions/list.ts

+		case "terraform":
+			return "Terraform 🏗️";
+		default:
+			return "Other";


Should we include the actual value for Other and Unknown below?

Suggested change

return "Other";

return `Other (${source})`;

I was thinking that too – will add

Did this in the next PR instead abc57e3

mrbbot · 2024-03-12T17:42:25Z

packages/wrangler/src/versions/view.ts

+		author_email: string;
+	};
+	annotations?: Record<string, string> & {
+		"workers/triggered_by"?: "upload" | string;


Should this have the other values, or just be string?

I extract it into another file in the next PR so will leave this for now

(I typed it from copy-pasting a api response so kept values for sanity knowing they'd widen into string anyway)

for versions view/list

RamIdeas requested a review from a team as a code owner March 8, 2024 14:40

RamIdeas force-pushed the versions-deploy-command branch from 28c06f0 to 724d2d4 Compare March 8, 2024 19:21

RamIdeas force-pushed the versions-list-command branch 2 times, most recently from d909fc7 to 439ca2d Compare March 8, 2024 19:37

mrbbot approved these changes Mar 12, 2024

View reviewed changes

RamIdeas force-pushed the versions-deploy-command branch from 19dbe5a to 96b38f7 Compare March 12, 2024 20:38

RamIdeas force-pushed the versions-list-command branch from f1b4772 to a608e35 Compare March 12, 2024 22:09

RamIdeas force-pushed the versions-deploy-command branch 3 times, most recently from 32b6d43 to b681d4e Compare March 12, 2024 22:37

RamIdeas force-pushed the versions-list-command branch 2 times, most recently from 2f6d5e8 to 1310333 Compare March 12, 2024 23:05

Base automatically changed from versions-deploy-command to main March 13, 2024 09:24

RamIdeas force-pushed the versions-list-command branch from 1310333 to 8c2292a Compare March 13, 2024 09:38

RamIdeas added 10 commits March 13, 2024 09:41

move versions subcommand registration into versions directory

c414dc8

implement versions list command

d9cc438

refactor to use shared util

60523e1

implement versions view command

043c1a1

add versions view tests

8553916

add versions list tests

cb33b10

remove unused imports

887f47a

only normalizeOutput in snapshot tests if necessary

e6d37d5

port Source display logic from legacy deployments command

3c3d744

for versions view/list

add changeset

efd3514

RamIdeas force-pushed the versions-list-command branch from 8c2292a to efd3514 Compare March 13, 2024 09:42

RamIdeas merged commit 4730b6c into main Mar 13, 2024
15 checks passed

RamIdeas deleted the versions-list-command branch March 13, 2024 09:54

workers-devprod mentioned this pull request Mar 13, 2024

Version Packages #5236

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `versions list` + `versions view` commands #5208

Implement `versions list` + `versions view` commands #5208

RamIdeas commented Mar 8, 2024 •

edited

changeset-bot bot commented Mar 8, 2024 •

edited

github-actions bot commented Mar 8, 2024 •

edited

codecov bot commented Mar 8, 2024 •

edited

mrbbot left a comment

mrbbot Mar 12, 2024

RamIdeas Mar 12, 2024

mrbbot Mar 12, 2024

mrbbot Mar 12, 2024

RamIdeas Mar 12, 2024

mrbbot Mar 12, 2024

RamIdeas Mar 12, 2024

RamIdeas Mar 12, 2024

mrbbot Mar 12, 2024

RamIdeas Mar 12, 2024 •

edited

		@@ -0,0 +1,22 @@
		import { logger } from "../logger";

		export default function renderLabelledValues(

Implement versions list + versions view commands #5208

Implement versions list + versions view commands #5208

Conversation

RamIdeas commented Mar 8, 2024 • edited

What this PR solves / how to test

Author has addressed the following

changeset-bot bot commented Mar 8, 2024 • edited

🦋 Changeset detected

github-actions bot commented Mar 8, 2024 • edited

codecov bot commented Mar 8, 2024 • edited

Codecov Report

mrbbot left a 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

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

Choose a reason for hiding this comment

RamIdeas Mar 12, 2024 • edited

Choose a reason for hiding this comment

Implement `versions list` + `versions view` commands #5208

Implement `versions list` + `versions view` commands #5208

RamIdeas commented Mar 8, 2024 •

edited

changeset-bot bot commented Mar 8, 2024 •

edited

github-actions bot commented Mar 8, 2024 •

edited

codecov bot commented Mar 8, 2024 •

edited

RamIdeas Mar 12, 2024 •

edited