Skip to content

Add new entrypoint option to --load-extension #1789

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 23, 2022
Merged

Add new entrypoint option to --load-extension #1789

merged 1 commit into from
Aug 23, 2022
+140 −2

Conversation

asg017
Copy link
Collaborator

@asg017 asg017 commented Aug 19, 2022

Closes #1784

The --load-extension flag can now accept an optional "entrypoint" value, to specify which entrypoint SQLite should load from the given extension.

# would load default entrypoint like before
datasette data.db --load-extension ext

# loads the extensions with the "sqlite3_foo_init" entrpoint
datasette data.db --load-extension ext:sqlite3_foo_init

# loads the extensions with the "sqlite3_bar_init" entrpoint
datasette data.db --load-extension ext:sqlite3_bar_init

For testing, I added a small SQLite extension in C at tests/ext.c. If compiled, then pytest will run the unit tests in test_load_extensions.pyto verify that Datasette loads in extensions correctly (and loads the correct entrypoints). Compiling the extension requires a C compiler, I compiled it on my Mac with:

gcc ext.c -I path/to/sqlite -fPIC -shared -o ext.dylib

Where path/to/sqlite is a directory that contains the SQLite amalgamation header files.

Re documentation: I added a bit to the help text for --load-extension (which I believe should auto-add to documentation?), and the existing extension documentation is spatialite specific. Let me know if a new extensions documentation page would be helpful!

@simonw
Copy link
Owner

simonw commented Aug 20, 2022

This looks great!

You need to run cog -r docs/*.rst to update the docs, then add those to the PR.

@simonw
Copy link
Owner

simonw commented Aug 20, 2022

If you cherry-pick in the fix from #1778 it should fix the Read the Docs failure.

@simonw
Copy link
Owner

simonw commented Aug 20, 2022

If you wanted to be really ambitious about documentation a page that explained what it takes to actually run SQLite extensions in Python would be great. I'm still trying to figure out the best way to do that in macOS myself.

@simonw
Copy link
Owner

simonw commented Aug 20, 2022

I don't think we should block landing this PR on that though.

@asg017 asg017 force-pushed the add-load-extension-entrypoint branch from 036c097 to 5a2a05f Compare August 21, 2022 16:12
@asg017
Copy link
Collaborator Author

asg017 commented Aug 21, 2022

Rebased, Read the docs failure should now now fixed

Re docs - ya that's a pretty ambitious page, I'm still not 100% sure what the best practices are/should be... Would be happy to make that page in a future PR

@simonw
Copy link
Owner

simonw commented Aug 21, 2022

Any tips on how I can get the GitHub Action test.yml workflow to compile this so that the test runs there?

I imagine I need an extra step before

datasette/.github/workflows/test.yml

Line 31 in 663ac43

- name: Run tests

Neatest thing would be to cache the compiled file between actions runs (in the Actions cache) so we don't have to add the SQLite build dependencies on most runs.

@asg017
Copy link
Collaborator Author

asg017 commented Aug 23, 2022

@simonw to build the extension on ubuntu, you can run:

apt-get update && apt-get install libsqlite3-dev gcc
gcc ext.c -fPIC -shared -o ext.so

I'm not the best with Actions, but if you set the cache key to ext.c, run those two commands to download dependencies + compile to ext.so, then the unit test should pick it up and run it correctly. Let me know if you want me to update the PR with that added

@simonw
Copy link
Owner

simonw commented Aug 23, 2022

I'm going to merge as-is and then figure out the GitHub Actions bit separately. Thanks!

@simonw simonw merged commit 1d64c9a into simonw:main Aug 23, 2022
simonw added a commit that referenced this pull request Aug 23, 2022
@simonw
Applied Black, refs #1789
fd1086c
simonw added a commit that referenced this pull request Aug 23, 2022
@simonw
Run the --load-extension test, refs #1789
25a8c32
simonw added a commit that referenced this pull request Aug 23, 2022
@simonw
Ran cog, refs #1789
456dc15
simonw added a commit that referenced this pull request Aug 23, 2022
@simonw
Ran cog, refs #1789
361914b
simonw added a commit that referenced this pull request Aug 23, 2022
@simonw
I don't think I need to apt-get install anything, refs #1789
02debab
@simonw
Copy link
Owner

simonw commented Aug 23, 2022

Looks like it's not actually necessary to apt-get install anything extra in order to compile that test extension:

image

@simonw simonw mentioned this pull request Aug 23, 2022
Merged
simonw added a commit that referenced this pull request Aug 24, 2022
@simonw
Test --load-extension in GitHub Actions (#1792)
ba35105 
* Run the --load-extension test, refs #1789
* Ran cog, refs #1789
@simonw simonw mentioned this pull request Aug 27, 2022
Closed
simonw added a commit that referenced this pull request Sep 26, 2022
@simonw
Release 0.63a0
212137a 
Refs #1786, #1787, #1789, #1794, #1800, #1804, #1805, #1808, #1809, #1816
@simonw simonw mentioned this pull request Sep 26, 2022
Closed
@simonw simonw mentioned this pull request Oct 27, 2022
Closed
simonw added a commit that referenced this pull request Oct 27, 2022
@simonw
Release 0.63
bf00b0b 
Refs #1646, #1786, #1787, #1789, #1794, #1800, #1804, #1805, #1808, #1809, #1816, #1819, #1825, #1829, #1831, #1834, #1844, #1853, #1860

Closes #1869
simonw added a commit that referenced this pull request Oct 28, 2022
@simonw
Release 0.63
61171f0 
Refs #1646, #1786, #1787, #1789, #1794, #1800, #1804, #1805, #1808, #1809, #1816, #1819, #1825, #1829, #1831, #1834, #1844, #1853, #1860

Closes #1869
@asg017 asg017 mentioned this pull request Mar 12, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Include "entrypoint" option on --load-extension?
2 participants
@asg017 @simonw