Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default D1 export to --local #7694

Merged
merged 2 commits into from
Jan 9, 2025
Merged

Conversation

joshthoward
Copy link
Contributor

@joshthoward joshthoward commented Jan 7, 2025

Copy behavior of wrangler d1 execute for wrangler d1 export.

  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because: Change is covered by unit tests
  • Public documentation

@joshthoward joshthoward requested review from a team as code owners January 7, 2025 21:49
Copy link

changeset-bot bot commented Jan 7, 2025

🦋 Changeset detected

Latest commit: 4791a5c

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

Copy link
Contributor

github-actions bot commented Jan 7, 2025

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/12678013135/npm-package-wrangler-7694

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

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

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12678013135/npm-package-wrangler-7694 dev path/to/script.js
Additional artifacts:
wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12678013135/npm-package-cloudflare-workers-bindings-extension-7694 -O ./cloudflare-workers-bindings-extension.0.0.0-vf43a39eab.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-vf43a39eab.vsix
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12678013135/npm-package-create-cloudflare-7694 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12678013135/npm-package-cloudflare-kv-asset-handler-7694
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12678013135/npm-package-miniflare-7694
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12678013135/npm-package-cloudflare-pages-shared-7694
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12678013135/npm-package-cloudflare-unenv-preset-7694
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12678013135/npm-package-cloudflare-vitest-pool-workers-7694
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12678013135/npm-package-cloudflare-workers-editor-shared-7694
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12678013135/npm-package-cloudflare-workers-shared-7694
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12678013135/npm-package-cloudflare-workflows-shared-7694

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


[email protected] includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20241230.0
workerd 1.20241230.0 1.20241230.0
workerd --version 1.20241230.0 2024-12-30

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

@joshthoward joshthoward force-pushed the joshthoward/d1-export-default-local branch from b7a4e3a to f5fc7f1 Compare January 7, 2025 22:04
@isaac-mcfadyen
Copy link
Contributor

IMO this is a very breaking change (all users using D1 export will suddenly be exporting their local DB instead of their remote DB).

Because of that, is this planned to be a major version bump? Landing this in a minor/patch has the potential to disrupt many user workflows.

@joshthoward
Copy link
Contributor Author

This is not a breaking change. This is a feature enhancement where if --local or --remote are not specified wrangler d1 export will default to local rather than failing with a user error. All existing uses will continue working. This will get a minor version bump.

@rozenmd
Copy link
Contributor

rozenmd commented Jan 8, 2025

To be clear, wrangler d1 export did not default to either local or remote. If you didn't specify, it would error:

% npx wrangler d1 export northwind --output ./test.sql

 ⛅️ wrangler 3.100.0
--------------------


✘ [ERROR] You must specify either --local or --remote

IMO this is a feat/minor bump since existing user code cannot already be relying on this behaviour (no one has scripts that intentionally error).

@joshthoward joshthoward force-pushed the joshthoward/d1-export-default-local branch from f5fc7f1 to 992a22d Compare January 8, 2025 19:54
@joshthoward joshthoward force-pushed the joshthoward/d1-export-default-local branch from 992a22d to 57a10b4 Compare January 8, 2025 19:54
@penalosa
Copy link
Contributor

penalosa commented Jan 9, 2025

For posterity, merging with a failing C3 check because this PR doesn't touch C3 and the same test is failing (flaking, probably) on main

@penalosa penalosa merged commit f3c2f69 into main Jan 9, 2025
29 of 30 checks passed
@penalosa penalosa deleted the joshthoward/d1-export-default-local branch January 9, 2025 11:18
@workers-devprod workers-devprod mentioned this pull request Jan 8, 2025
@MORTEZAMIRSALI
Copy link

2c76887

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants