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

Migrate driver adapters to prisma/prisma #4380

Merged
merged 30 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
9bc500f
Promote connector-test-kit to the driver-adapters directory and remov…
miguelff Oct 24, 2023
86b585e
Remove node_modules from connector-test-kit-executor
miguelff Oct 24, 2023
a93c6f7
Remove dist from connector-test-kit-executor
miguelff Oct 24, 2023
4e6db6e
Ignore non-relevant files
miguelff Oct 24, 2023
d3d9ad4
Sort out dependencies
miguelff Oct 24, 2023
269d2dc
Makefile to setup driver adapters from a checkout of prisma
miguelff Oct 24, 2023
1490e5b
Only clone prisma/prisma shallowly
miguelff Oct 24, 2023
254f5b4
Delete driver-adapter-smoke-tests.yml
miguelff Oct 24, 2023
cba8e8f
Merge remote-tracking branch 'origin/main' into driver-adapters-migra…
miguelff Oct 24, 2023
03913ca
DRIVER_ADAPTERS_BRANCH=driver-adapters-migration see if this works
miguelff Oct 25, 2023
e0ea528
DRIVER_ADAPTERS_BRANCH=driver-adapters-migration change deprecated se…
miguelff Oct 25, 2023
463775a
DRIVER_ADAPTERS_BRANCH=driver-adapters-migration tmp remove
miguelff Oct 25, 2023
418c50d
DRIVER_ADAPTERS_BRANCH=driver-adapters-migration clearer detection of…
miguelff Oct 25, 2023
67a89bd
DRIVER_ADAPTERS_BRANCH=driver-adapters-migration Build executor separ…
miguelff Oct 25, 2023
6d070ed
DRIVER_ADAPTERS_BRANCH=driver-adapters-migration Add make tasks to te…
miguelff Oct 25, 2023
5c2601a
Document and ease running driver adapter tests
miguelff Oct 25, 2023
96d4cd0
Revert "DRIVER_ADAPTERS_BRANCH=driver-adapters-migration tmp remove"
miguelff Oct 25, 2023
ca8b9e3
Move documentation to where it belongs
miguelff Oct 25, 2023
06cb9c8
Document how to do integration testing in shorter loops in CI.
miguelff Oct 25, 2023
0c2f426
chore(driver-adapters): remove outdated symlink to tsconfig file
jkomyno Oct 26, 2023
d6614c1
fix(driver-adapters): use ws, making connector-test-kit-executor comp…
jkomyno Oct 26, 2023
187e46f
fix(driver-adapters): remove warning "import.meta" is not available w…
jkomyno Oct 26, 2023
eeaaa8f
chore(driver-adapters): remove references to query-engine-driver-adap…
jkomyno Oct 26, 2023
5541534
chore: merge main, fix conflicts
jkomyno Oct 26, 2023
0c2ab39
Merge branch 'main' into driver-adapters-migration
Oct 26, 2023
2ef6ee3
Revert "chore(driver-adapters): remove references to query-engine-dri…
miguelff Oct 26, 2023
a0b1f28
Remove publish-driver-adapters workflow
miguelff Oct 26, 2023
feb5244
Fix using main branch
miguelff Oct 27, 2023
489fe86
Take back conditional on docker login after bad main merge
miguelff Oct 27, 2023
7eb2831
Merge branch 'main' into driver-adapters-migration
Oct 27, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 0 additions & 131 deletions .github/workflows/driver-adapter-smoke-tests.yml

This file was deleted.

11 changes: 11 additions & 0 deletions .github/workflows/query-engine-driver-adapters.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}

- name: 'Setup Node.js'
uses: actions/setup-node@v3
Expand All @@ -72,6 +74,15 @@ jobs:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}

- name: Extract Branch Name
id: extract-branch
run: |
branch="$(git show -s --format=%s | grep -o "DRIVER_ADAPTERS_BRANCH=[^ ]*" | cut -f2 -d=)"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows us to test the query engine code against a particular prisma/prisma branch in CI. So we are not depending on merge ordering of prisma/prisma changes.

Example:

  • Feature requires changes in the engine and driver-adapters
  • changes in driver adapters are in branch foo of prisma/prisma
  • If we want to test a particular commit in the engines against those changes we just need to push a commit in here containing DRIVER_ADAPTERS_BRANCH=foo.

if [ -n $branch ]; then
echo "Using $branch branch of driver adapters"
echo "DRIVER_ADAPTERS_BRANCH=$branch" >> "$GITHUB_ENV"
fi

- run: make ${{ matrix.adapter.setup_task }}

- uses: dtolnay/rust-toolchain@stable
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,6 @@ dmmf.json
graph.dot

prisma-schema-wasm/nodejs

# This symlink looks orphan here, but it comes from prisma/prisma where driver adapters reference a file in their parent directory
tsconfig.build.adapter.json
56 changes: 54 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ CONFIG_PATH = ./query-engine/connector-test-kit-rs/test-configs
CONFIG_FILE = .test_config
SCHEMA_EXAMPLES_PATH = ./query-engine/example_schemas
DEV_SCHEMA_FILE = dev_datamodel.prisma
DRIVER_ADAPTERS_BRANCH ?= main

LIBRARY_EXT := $(shell \
case "$$(uname -s)" in \
Expand Down Expand Up @@ -44,7 +45,13 @@ release:
#################

test-qe:
ifndef DRIVER_ADAPTER
cargo test --package query-engine-tests
else
@echo "Executing query engine tests with $(DRIVER_ADAPTER) driver adapter"; \
# Add your actual command for the "test-driver-adapter" task here
$(MAKE) test-driver-adapter-$(DRIVER_ADAPTER);
endif

test-qe-verbose:
cargo test --package query-engine-tests -- --nocapture
Expand Down Expand Up @@ -80,6 +87,10 @@ dev-sqlite:
dev-libsql-sqlite: build-qe-napi build-connector-kit-js
cp $(CONFIG_PATH)/libsql-sqlite $(CONFIG_FILE)

test-libsql-sqlite: dev-libsql-sqlite test-qe-st

test-driver-adapter-libsql: test-libsql-sqlite
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convention over configuration to run driver adapters tests easily. See README.md changes.


start-postgres9:
docker compose -f docker-compose.yml up --wait -d --remove-orphans postgres9

Expand Down Expand Up @@ -115,12 +126,20 @@ start-pg-postgres13: build-qe-napi build-connector-kit-js start-postgres13
dev-pg-postgres13: start-pg-postgres13
cp $(CONFIG_PATH)/pg-postgres13 $(CONFIG_FILE)

test-pg-postgres13: dev-pg-postgres13 test-qe-st

test-driver-adapter-pg: test-pg-postgres13

start-neon-postgres13: build-qe-napi build-connector-kit-js
docker compose -f docker-compose.yml up --wait -d --remove-orphans neon-postgres13

dev-neon-ws-postgres13: start-neon-postgres13
cp $(CONFIG_PATH)/neon-ws-postgres13 $(CONFIG_FILE)

test-neon-ws-postgres13: dev-neon-ws-postgres13 test-qe-st

test-driver-adapter-neon: test-neon-ws-postgres13

start-postgres14:
docker compose -f docker-compose.yml up --wait -d --remove-orphans postgres14

Expand Down Expand Up @@ -255,15 +274,48 @@ start-planetscale-vitess8: build-qe-napi build-connector-kit-js
dev-planetscale-vitess8: start-planetscale-vitess8
cp $(CONFIG_PATH)/planetscale-vitess8 $(CONFIG_FILE)

test-planetscale-vitess8: dev-planetscale-vitess8 test-qe-st

test-driver-adapter-planetscale: test-planetscale-vitess8

######################
# Local dev commands #
######################

build-qe-napi:
cargo build --package query-engine-node-api

build-connector-kit-js:
cd query-engine/driver-adapters/js && pnpm i && pnpm build
build-connector-kit-js: build-driver-adapters symlink-driver-adapters
cd query-engine/driver-adapters/connector-test-kit-executor && pnpm i && pnpm build

build-driver-adapters: ensure-prisma-present
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We ensure prisma/prisma is our FS, and build driver adapters from source.

@echo "Building driver adapters..."
@cd ../prisma && pnpm --filter "*adapter*" i && pnpm --filter "*adapter*" build
@echo "Driver adapters build completed.";

symlink-driver-adapters: ensure-prisma-present
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Then we symlink driver adapters code

@echo "Creating symbolic links for driver adapters..."
@for dir in $(wildcard $(realpath ../prisma)/packages/*adapter*); do \
if [ -d "$$dir" ]; then \
dir_name=$$(basename "$$dir"); \
ln -sfn "$$dir" "$(realpath .)/query-engine/driver-adapters/$$dir_name"; \
echo "Created symbolic link for $$dir_name"; \
fi; \
done;
@ln -sfn "../prisma/tsconfig.build.adapter.json" "./tsconfig.build.adapter.json"; \
echo "Symbolic links creation completed.";

ensure-prisma-present:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. If locally we have a clone of prisma, we are done, if that clone diverges from main we will warn the user. If we don´t have a clone (f.i. in CI) we shallowly clone it -- takes less than 10 seconds

@if [ -d ../prisma ]; then \
cd "$(realpath ../prisma)" && git fetch origin main; \
LOCAL_CHANGES=$$(git diff --name-only HEAD origin/main -- 'packages/*adapter*'); \
if [ -n "$$LOCAL_CHANGES" ]; then \
echo "⚠️ ../prisma diverges from prisma/prisma main branch. Test results might diverge from those in CI ⚠️ "; \
fi \
else \
echo "git clone --depth=1 https://github.com/prisma/prisma.git --branch=$(DRIVER_ADAPTERS_BRANCH) ../prisma"; \
git clone --depth=1 https://github.com/prisma/prisma.git --branch=$(DRIVER_ADAPTERS_BRANCH) "../prisma" && echo "Prisma repository has been cloned to ../prisma"; \
fi;

# Quick schema validation of whatever you have in the dev_datamodel.prisma file.
validate:
Expand Down
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ integration tests.
- Alternatively: Load the defined environment in `./.envrc` manually in your shell.

**Setup:**

There are helper `make` commands to set up a test environment for a specific
database connector you want to test. The commands set up a container (if needed)
and write the `.test_config` file, which is picked up by the integration
Expand Down Expand Up @@ -234,6 +235,21 @@ Other variables may or may not be useful.

Run `cargo test` in the repository root.

#### Testing driver adapters

Driver adapters are a feature to run queries through javascript drivers from rust. An adapter for a certain driver is
provided by the prisma client and then use by rust code. If you want to run query engine tests through driver adapters:

- `DRIVER_ADAPTER=$adapter make qe-test`

Where `$adapter` is one of `pg`, `neon`, or `planetscale` or `libsql` (the driver adapters currently supported)

This make task hides the underlying complexity of spawning the right docker containers, pulling driver adapters code
from prisma/prisma, building them, and build a test runner to use them.

When pulling the driver adapters code, make will ensure you have a clone of prisma/prisma in the same directory as
prisma-engines. If you don't, it will clone it for you.

## Parallel rust-analyzer builds

When rust-analzyer runs `cargo check` it will lock the build directory and stop any cargo commands from running until it has completed. This makes the build process feel a lot longer. It is possible to avoid this by setting a different build path for
Expand Down
7 changes: 4 additions & 3 deletions query-engine/connector-test-kit-rs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ If you choose to set up the databases yourself, please note that the connection

#### Running tests through driver adapters

The query engine is able to delegate query execution to javascript through [driver adapters](query-engine/driver-adapters/js/README.md).
This means that instead of drivers being implemented in Rust, it's a layer of adapters over NodeJs drivers the code that actually communicates with the databases.
The query engine is able to delegate query execution to javascript through driver adapters.
This means that instead of drivers being implemented in Rust, it's a layer of adapters over NodeJs
drivers the code that actually communicates with the databases. See [`adapter-*` packages in prisma/prisma](https://github.com/prisma/prisma/tree/main/packages)

To run tests through a driver adapters, you should also configure the following environment variables:

Expand All @@ -78,7 +79,7 @@ To run tests through a driver adapters, you should also configure the following
Example:

```shell
export EXTERNAL_TEST_EXECUTOR="$WORKSPACE_ROOT/query-engine/driver-adapters/js/connector-test-kit-executor/script/start_node.sh"
export EXTERNAL_TEST_EXECUTOR="$WORKSPACE_ROOT/query-engine/driver-adapters/connector-test-kit-executor/script/start_node.sh"
export DRIVER_ADAPTER=neon
export DRIVER_ADAPTER_CONFIG ='{ "proxyUrl": "127.0.0.1:5488/v1" }'
````
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ impl TestConfig {
/// and the workspace_root is set, then use the default external test executor.
fn fill_defaults(&mut self) {
const DEFAULT_TEST_EXECUTOR: &str =
"query-engine/driver-adapters/js/connector-test-kit-executor/script/start_node.sh";
"query-engine/driver-adapters/connector-test-kit-executor/script/start_node.sh";

if self
.external_test_executor
Expand Down
3 changes: 3 additions & 0 deletions query-engine/driver-adapters/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
node_modules
adapter-*
driver-adapter-utils
Comment on lines +1 to +3
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignoring symlinks from prisma/prisma

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
node_modules
pnpm-debug.log
dist/
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
{
"engines": {
"node": ">=16.13",
"pnpm": ">=8.6.6 <9"
},
"name": "connector-test-kit-executor",
"version": "5.4.0",
"version": "0.0.1",
"description": "",
"main": "dist/index.js",
"private": true,
Expand All @@ -16,13 +20,19 @@
"@libsql/client": "0.3.5",
"@neondatabase/serverless": "^0.6.0",
"@planetscale/database": "1.11.0",
"@prisma/adapter-libsql": "workspace:*",
"@prisma/adapter-neon": "workspace:*",
"@prisma/adapter-pg": "workspace:*",
"@prisma/adapter-planetscale": "workspace:*",
"@prisma/driver-adapter-utils": "workspace:*",
"@prisma/adapter-libsql": "../adapter-libsql",
"@prisma/adapter-neon": "../adapter-neon",
"@prisma/adapter-pg": "../adapter-pg",
"@prisma/adapter-planetscale": "../adapter-planetscale",
"@prisma/driver-adapter-utils": "../driver-adapter-utils",
Comment on lines +24 to +28
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on symlinked driver adapters

"@types/pg": "^8.10.2",
"pg": "^8.11.3",
"undici": "^5.26.2"
},
"devDependencies": {
"@types/node": "^20.5.1",
"tsup": "^7.2.0",
"tsx": "^3.12.7",
"typescript": "^5.1.6"
}
}
}
Loading