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

feat: flight SQL sdk #29

Merged
merged 9 commits into from
Nov 19, 2024
Merged

feat: flight SQL sdk #29

merged 9 commits into from
Nov 19, 2024

Conversation

dav1do
Copy link
Collaborator

@dav1do dav1do commented Nov 13, 2024

This creates a new flight-sql-client package that uses napi to generate N-API bindings for doing the grpc/protobuf flight things and communicating with the ceramic-one flight SQL server. Currently targets #30, which are things I encountered while implementing this and extracted as they're not exactly related.

I'm planning to implement package publishing in a follow up branch. I could also use help figuring out the typescript package structure to see if we can add new files that are available. Currently, napi generates index.js and index.d.ts in the root, which can be overridden via the --js and --dts flags, but I haven't quite managed to get the build/exports quite right so far.

  • Modified the build to compile the N-API binaries for platforms we support. Sourced from the napi package template but modified for pnpm and our purposes.
  • Still need to figure out how to publish the npm packages and make sure they actually work. Currently during CI, the artifacts are downloaded on different platforms, dropped into the packages/flight-sql-client folder, and the package tests are run which simply gets the version from the binary to make sure it's the appropriate architecture.
    • When publishing the sub-packages, they should simply be the package.json, the README.md, and the binary-arch.node file. The napi artifacts command will move files from a directory to the target packages (artifacts and npm/* by default`).

@dav1do dav1do force-pushed the feat/flight-sql-sdk branch 10 times, most recently from 8e736fc to 780e241 Compare November 15, 2024 18:23
@dav1do dav1do force-pushed the feat/flight-sql-sdk branch from 780e241 to 858cc8b Compare November 15, 2024 21:54
@dav1do dav1do changed the base branch from main to chore/rework-build November 15, 2024 21:54
@dav1do dav1do force-pushed the feat/flight-sql-sdk branch 2 times, most recently from f3f8f11 to 7ea609d Compare November 15, 2024 22:42
@dav1do dav1do marked this pull request as ready for review November 15, 2024 22:58
@dav1do dav1do force-pushed the feat/flight-sql-sdk branch from 04730d1 to 7803e0d Compare November 16, 2024 19:13
@mzkrasner
Copy link
Contributor

Looks good on my end - pulled and tried locally.

Base automatically changed from chore/rework-build to main November 19, 2024 19:52
only targeting platforms that ceramic-one runs on currently
- integration tests are only run on ubuntu VM currently
- still need to get artifacts in the right place to publish
need to figure out how to get the imports right with the generated code.
might need to use a sub package or two side by side packages...or just learn more about typescript
pnpm was picking up target and a ton of extra things
this is more clear, plus we're actually using the N-API interface and not WASM
@dav1do dav1do force-pushed the feat/flight-sql-sdk branch from cffb558 to 7f56ff4 Compare November 19, 2024 19:54
@dav1do dav1do merged commit 1f50cd9 into main Nov 19, 2024
9 checks passed
@dav1do dav1do deleted the feat/flight-sql-sdk branch November 19, 2024 20:03
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.

3 participants