-
Notifications
You must be signed in to change notification settings - Fork 304
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
Extend SqliteDatabase with a reset() method that recreates the database #2680
Conversation
deb6d4e
to
7198dad
Compare
// Delete all: increments start over | ||
await doReq('deleteAll'); | ||
assert.equal(await doReq('increment'), 1); | ||
assert.equal(await doReq('increment'), 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: a variation on this that also waits on these concurrently would be nice.
const results = await Promise.all([
doReq('deleteAll'),
doReq('increment'),
doReq('increment'),
]);
assert.deepStrictEqual(results, ...);
Largely to make sure that operations are correctly sequenced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand... what would that test that is interesting here?
97d14b5
to
b97ec2a
Compare
Note that `deleteAll()` has never been possible to perform as part of a transaction in production, so this should not be considered a breaking change. Also this particular code is not (yet) reachable in production anyway.
This can be used to re-enable WAL mode, and other things that need to happen at startup.
…bject" This log has served its purpose (it turns out people do, in fact, call deleteAll() with an alarm set fairly often). It is also getting in the way of deleteAll() for SRS-backed objects since they don't implement alarms yet so the getAlarm() call throws. This reverts commit 2f13452.
This doesn't really matter for workerd, where the commit callback is a no-op. It matters in production, though.
b97ec2a
to
cf8c335
Compare
And use this to implement deleteAll().
Thus,
deleteAll()
will now delete SQL tables as well, instead of just KV data.