fix: cleans up `d1 list` output for json #2622

rozenmd · 2023-01-25T13:45:23Z

This PR hides the warning messages, and just returns clean json when running wrangler d1 list --json

Before:

rozenmd@cflaptop test % npx wrangler d1 list
--------------------
🚧 D1 is currently in open alpha and is not recommended for production data and traffic
🚧 Please report any bugs to https://github.com/cloudflare/wrangler2/issues/new/choose
🚧 To request features, visit https://community.cloudflare.com/c/developers/d1
🚧 To give feedback, visit https://discord.gg/cloudflaredev
--------------------

┌──────────────────────────────┬─────────────────┐
│ uuid                         │ name            │
├──────────────────────────────┼─────────────────┤
│ xxxxxx-xxxx-xxxx-xxxx-xxxxxx │ test            │
├──────────────────────────────┼─────────────────┤
│ xxxxxx-xxxx-xxxx-xxxx-xxxxxx │ test2           │
├──────────────────────────────┼─────────────────┤
│ xxxxxx-xxxx-xxxx-xxxx-xxxxxx │ test3           │
└──────────────────────────────┴─────────────────┘

After:

rozenmd@cflaptop test % npx wrangler d1 list --json
[
  {
    "uuid": "xxxxxx-xxxx-xxxx-xxxx-xxxxxx",
    "name": "test"
  },
  {
    "uuid": "xxxxxx-xxxx-xxxx-xxxx-xxxxxx",
    "name": "test2"
  },
  {
    "uuid": "xxxxxx-xxxx-xxxx-xxxx-xxxxxx",
    "name": "test3"
  },
]

Closes #2323

changeset-bot · 2023-01-25T13:45:27Z

🦋 Changeset detected

Latest commit: 44e85d1

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

This PR includes changesets to release 1 package

Name	Type
wrangler	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

codecov · 2023-01-25T14:39:03Z

Codecov Report

Merging #2622 (44e85d1) into main (416babf) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main    #2622   +/-   ##
=======================================
  Coverage   73.19%   73.20%           
=======================================
  Files         159      159           
  Lines        9847     9850    +3     
  Branches     2622     2624    +2     
=======================================
+ Hits         7208     7211    +3     
  Misses       2639     2639

Impacted Files	Coverage Δ
packages/wrangler/src/d1/list.tsx	`26.66% <0.00%> (-2.97%)`	⬇️
packages/wrangler/src/git-client.ts	`81.25% <0.00%> (+4.16%)`	⬆️
...ackages/wrangler/src/__tests__/helpers/mock-bin.ts	`100.00% <0.00%> (+5.26%)`	⬆️

JacobMGEvans

It's looking good, looks like moving some stuff around and JSON option feature too.

The blocking comment is in regards to the beta warning message.

JacobMGEvans · 2023-01-25T15:29:34Z

packages/wrangler/src/d1/list.tsx

+	if (json) {
+		logger.log(JSON.stringify(dbs, null, 2));
+	} else {
+		logger.log(d1BetaWarning);


Should this beta warning be moved above the if statement?

The point of the pull is to have a json parseable output - by having the warning, you lose that ability

My idea was to not print the warning since it might pollute stdout

Ok I see the use case you may be referring to in regards of maybe piping the stdout to say a .json file or something and this messaging would break that.

packages/wrangler/src/d1/list.tsx

github-actions · 2023-01-25T15:50:47Z

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/runs/4007970845/npm-package-wrangler-2622

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

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

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/runs/4007970845/npm-package-wrangler-2622 dev path/to/script.js

Additional artifacts:

npm install https://prerelease-registry.devprod.cloudflare.dev/runs/4007970845/npm-package-cloudflare-pages-shared-2622

JacobMGEvans

Appreciate the clarification of the use case. We want to be considerate of the messaging as part of the Wrangler Public API, I think you make a good point that the messaging is strongly prominent in other parts of d1 usage

JacobMGEvans · 2023-01-25T17:49:03Z

packages/wrangler/src/d1/list.tsx

+	if (json) {
+		logger.log(JSON.stringify(dbs, null, 2));
+	} else {
+		logger.log(d1BetaWarning);


Ok I see the use case you may be referring to in regards of maybe piping the stdout to say a .json file or something and this messaging would break that.

rozenmd · 2023-01-25T18:04:38Z

Apologies @JacobMGEvans, I should've linked the GitHub issue: #2323

JacobMGEvans · 2023-01-25T18:22:30Z

Apologies @JacobMGEvans, I should've linked the GitHub issue: #2323

Looks like you did in the PR description! That's a miss on my part 😅

rozenmd · 2023-01-25T18:29:02Z

Added it after your last comment lol

fix: cleans up d1 list output for json

1622377

rozenmd self-assigned this Jan 25, 2023

rozenmd requested a review from a team as a code owner January 25, 2023 13:45

add changeset, move tests

53b2ea2

JacobMGEvans suggested changes Jan 25, 2023

View reviewed changes

rozenmd requested a review from JacobMGEvans January 25, 2023 16:24

rozenmd added 3 commits January 25, 2023 17:24

address PR feedback

8283f67

Update rare-laws-sing.md

daeb76e

Update rare-laws-sing.md

44e85d1

JacobMGEvans approved these changes Jan 25, 2023

View reviewed changes

rozenmd merged commit 9778b33 into main Jan 25, 2023

rozenmd deleted the rozenmd/d1-json-output branch January 25, 2023 18:05

github-actions bot mentioned this pull request Jan 25, 2023

Version Packages #2595

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: cleans up `d1 list` output for json #2622

fix: cleans up `d1 list` output for json #2622

rozenmd commented Jan 25, 2023 •

edited

Loading

changeset-bot bot commented Jan 25, 2023 •

edited

Loading

codecov bot commented Jan 25, 2023 •

edited

Loading

JacobMGEvans left a comment •

edited

Loading

JacobMGEvans Jan 25, 2023

Skye-31 Jan 25, 2023

rozenmd Jan 25, 2023

JacobMGEvans Jan 25, 2023

github-actions bot commented Jan 25, 2023 •

edited

Loading

JacobMGEvans left a comment

JacobMGEvans Jan 25, 2023

rozenmd commented Jan 25, 2023

JacobMGEvans commented Jan 25, 2023

rozenmd commented Jan 25, 2023

fix: cleans up d1 list output for json #2622

fix: cleans up d1 list output for json #2622

Conversation

rozenmd commented Jan 25, 2023 • edited Loading

changeset-bot bot commented Jan 25, 2023 • edited Loading

🦋 Changeset detected

codecov bot commented Jan 25, 2023 • edited Loading

Codecov Report

JacobMGEvans left a comment • edited Loading

Choose a reason for hiding this comment

JacobMGEvans Jan 25, 2023

Choose a reason for hiding this comment

Skye-31 Jan 25, 2023

Choose a reason for hiding this comment

rozenmd Jan 25, 2023

Choose a reason for hiding this comment

JacobMGEvans Jan 25, 2023

Choose a reason for hiding this comment

github-actions bot commented Jan 25, 2023 • edited Loading

JacobMGEvans left a comment

Choose a reason for hiding this comment

JacobMGEvans Jan 25, 2023

Choose a reason for hiding this comment

rozenmd commented Jan 25, 2023

JacobMGEvans commented Jan 25, 2023

rozenmd commented Jan 25, 2023

fix: cleans up `d1 list` output for json #2622

fix: cleans up `d1 list` output for json #2622

rozenmd commented Jan 25, 2023 •

edited

Loading

changeset-bot bot commented Jan 25, 2023 •

edited

Loading

codecov bot commented Jan 25, 2023 •

edited

Loading

JacobMGEvans left a comment •

edited

Loading

github-actions bot commented Jan 25, 2023 •

edited

Loading