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

Add readonly option to Better-SQLite3 #5530

Merged

Conversation

undees
Copy link
Contributor

@undees undees commented Mar 28, 2023

Better-SQLite3 supports a readonly option on initialization. This PR lets Knex users set that option.

(The automatic code formatter wanted to reflow some lines unrelated to the new code. I've split those automated changes out into a separate commit for clarity and to preserve author attribution on the changed lines.)

Docs PR is in knex/documentation#505.

@undees undees changed the title Add readonly option to better sqlite3 Add readonly option to Better-SQLite3 Mar 28, 2023
@undees undees force-pushed the add-readonly-option-to-better-sqlite3 branch from 9a01254 to 68578c8 Compare March 28, 2023 23:25
@coveralls
Copy link

Coverage Status

Coverage: 92.385% (-0.007%) from 92.392% when pulling 68578c8 on undees:add-readonly-option-to-better-sqlite3 into de2a8a2 on knex:master.

test/unit/dialects/better-sqlite3.js Show resolved Hide resolved
test/unit/dialects/better-sqlite3.js Outdated Show resolved Hide resolved
@undees undees force-pushed the add-readonly-option-to-better-sqlite3 branch from 68578c8 to 40764a2 Compare March 29, 2023 12:23
@OlivierCavadenti
Copy link
Collaborator

OlivierCavadenti commented Apr 3, 2023

Actions seems broken and we have no checks, I will try to fix that before merging.

@OlivierCavadenti OlivierCavadenti merged commit dab051b into knex:master Jun 25, 2023
@OlivierCavadenti
Copy link
Collaborator

Hi,

I have this error in tests, it's seems I can create a database with readonly: false (like before) but with readonly : true;

SqliteError: unable to open database file
    at new Database (node_modules\better-sqlite3\lib\database.js:63:26)
    at Client_BetterSQLite3.acquireRawConnection (lib\dialects\better-sqlite3\index.js:14:12)
    at create (lib\client.js:254:39)

@undees

@undees undees deleted the add-readonly-option-to-better-sqlite3 branch June 26, 2023 20:09
@undees
Copy link
Contributor Author

undees commented Jun 26, 2023

Hi, @OlivierCavadenti. Sorry about this! Looks like the tests were only passing intermittently, depending on whether a previous test case created test/unit/test.sqlite3 before the read-only case looked for it. I've opened #5608 to fix the broken test case, and just for good measure I (re-)verified manually in Node that Better-SQLite3 respects the readonly option.

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

Successfully merging this pull request may close these issues.

None yet

3 participants