mirror of
https://github.com/vectorize-io/hindsight.git
synced 2026-09-14 19:31:49 +08:00
d39a2ca618
* feat(oracle): unify migrations under Alembic with dialect dispatcher
Oracle DDL was a 636-line idempotent file (`migrations_oracle.py`) outside
Alembic, which meant no version tracking, no per-tenant version table, and
schema drift every time a PG migration was added without a corresponding
Oracle change. This unifies both backends behind a single Alembic tree.
- New `alembic/_dialect.py::run_for_dialect(pg=, oracle=)` helper. Each
migration declares `_pg_upgrade` / `_oracle_upgrade` and dispatches based
on the live connection's dialect.
- `alembic/env.py` is dialect-aware: PG keeps the existing search_path /
read-write session setup; Oracle uses `ALTER SESSION SET CURRENT_SCHEMA`
and `DDL_LOCK_TIMEOUT`.
- `alembic/script.py.mako` scaffolds the new pattern by default.
- All 59 existing PG migrations refactored mechanically — bodies moved into
`_pg_upgrade` / `_pg_downgrade`, top-level dispatchers added.
- New `o1a2b3c4d5e6_oracle_baseline` migration brings a fresh Oracle 23ai
database to the current schema in one step (PG = no-op). Drops the legacy
partition-conversion / dedup / `observation_sources` backfill since those
only existed for pre-baseline Oracle installs we explicitly are not
supporting.
- `OracleBackend.run_migrations()` now goes through the unified Alembic
pipeline; `migrations.py` skips the PG-specific advisory lock + pgvector
setup when the URL is Oracle.
- `migrations_oracle.py` deleted; tests updated to use `run_migrations()`.
- New `tests/test_migration_shape.py` lint fails CI if any migration omits
`run_for_dialect` — keeps drift from re-emerging.
- CLAUDE.md updated with the new template and dialect-asymmetry guidance.
* ci: run client integration tests against Oracle on oracle-tests label
Adds test-python-client-oracle and test-typescript-client-oracle. These
mirror the existing test-python-client / test-typescript-client jobs but
spin up Oracle 23ai as a service container and point the API server at it
via HINDSIGHT_API_DATABASE_BACKEND=oracle + DATABASE_URL.
Why a new job instead of matrixing the existing one: Oracle Free's image
takes ~2min to start and is network-heavy, so we don't want to pay that
cost on every PR — only when oracle-tests is opted in via the PR label,
matching the existing test-api-oracle gate.
Why client tests, not unit tests: the unit suite already runs against
both backends via the abstraction layer. Only the client tests exercise
full HTTP round-trips with real serialized payloads, so they catch API
changes that work on PG but break on Oracle (or vice versa) in ways the
abstraction can't see.
* refactor(oracle): tighten feature requirements and dedup is_oracle_url
- Move is_oracle_url to db_url.py and import from there in env.py and
migrations.py — was duplicated in both.
- Type-annotate _configure_pg_session / _configure_oracle_session params
(Engine, Connection); ty checks pass.
- Update the Oracle baseline comment around vector + text index creation
to make the hard requirement explicit: VECTOR + CTXSYS must be
available, the migration fails hard if either is missing. The
swallow-only-ORA-00955 behavior was already correct; the previous
comment misleadingly called it "best-effort".
* chore(openclaw): apply pending prettier reformat to keep verify-generated-files green
Three formatting-only changes prettier wants to make. They've been stale
on main; CI's verify-generated-files runs lint with LINT_ALL=1 (vs the
"only changed integrations" local default), which surfaces them on every
unrelated PR. Folding them in here so this PR can land.
* fix(retain): plumb ops through handle_document_tracking
Line 312 of fact_storage.py references ``ops`` without ``handle_document_tracking``
declaring it as a parameter — straight NameError on every retain that walks
the upsert path. Bug landed on main in d8ec2d7f (#1325) when
``delete_stale_observations_for_memories`` started taking a backend-aware
``ops`` to choose between the PG array operator and the Oracle junction
table; the call site was added but the parameter wasn't threaded into the
enclosing function.
Fix: add ``ops=None`` to ``handle_document_tracking`` and pass ``pool.ops``
from each of the three call sites in orchestrator.py.
This is unrelated to the Alembic dialect-dispatcher refactor in this PR but
is what's blocking it — the NameError caused 17 retain tests to fail (and
left a pytest-xdist worker in a state that hung the whole job at 99%).
* test(observation): pass ops to handle_document_tracking in upsert test
The test calls fact_storage.handle_document_tracking directly, which
delegates to delete_stale_observations_for_memories(ops=ops). With ops=None
the helper falls back to the Oracle junction-table query and fails on PG
with "relation public.observation_sources does not exist". Real callers
(orchestrator, _delete_stale_observations_for_memories wrapper) all pass
self._backend.ops; the test just needs to do the same.
* ci: run client-against-oracle on every API change, drop label gate
Reserve the "oracle-tests" label for the heavy test-api-oracle (full unit
suite). The two client integration jobs against Oracle should run on every
API/client change just like their PG counterparts — the whole point is to
catch PG/Oracle drift before merge, which doesn't work if you have to
remember to label every PR. test-api-oracle keeps its label gate because
the full suite is too slow to run on every push.
* fix(oracle): rewrite path-style service to ?service_name= for SQLAlchemy
Oracle Free / Autonomous DB only register a service name with the listener,
but SQLAlchemy's oracle+oracledb dialect interprets the URL path as a SID.
That mismatch crashes alembic migrations on first connect:
DPY-6003: SID "FREEPDB1" is not registered with the listener
Rewrite ``oracle://user:pass@host:port/SERVICE`` to
``oracle+oracledb://user:pass@host:port/?service_name=SERVICE`` so the
dialect uses the correct connect descriptor. ``?sid=`` and ``?service_name=``
already in the URL are passed through untouched.
Also adds scripts/dev/start-oracle.sh / stop-oracle.sh that spin up the same
Oracle 23ai Free image CI uses (``container-registry.oracle.com/database/free``)
and bootstrap the HINDSIGHT_TEST user, so we can repro this kind of issue
locally without round-tripping through GitHub Actions.
* fix(oracle): commit after migrations so alembic_version persists
On Oracle, alembic runs each migration with transactional_ddl=False
("Will assume non-transactional DDL"). Each CREATE TABLE auto-commits, but
the trailing ``UPDATE alembic_version SET version_num = ...`` is plain DML
that needs an explicit COMMIT. Without it the connection close rolls the
update back, leaving the schema fully created but the version row one
revision behind — so ``run_migrations`` reports success while the head row
sits at the previous revision.
Caught locally with the new scripts/dev/start-oracle.sh harness running the
same Oracle 23ai Free image CI uses; alembic_version was stuck at
``k6l7m8n9o0p1`` even though every table from the ``o1a2b3c4d5e6`` baseline
existed. After the fix it correctly advances to ``o1a2b3c4d5e6``, and a
second run is a no-op as expected.
PG already needs the same commit (Supabase RW-mode SET), so just drop the
``if not is_oracle`` guard.
* ci(oracle): run python client tests sequentially to avoid ORA-00060
The python client pyproject.toml defaults to -n auto (pytest-xdist).
Against Oracle that hits row-level deadlocks during retain cleanup —
ORA-00060 is logged repeatedly in the API server output and most tests
fail with "Internal Server Error" at fixture teardown. Same shape as the
existing test-api-oracle issue, which is already pinned to -n0.
Override to -n0 in the Oracle client job (only). The PG client job stays
parallel since pgvector + advisory locks handle concurrent retain fine.
TS client tests are unaffected — they run via vitest, not pytest.
124 lines
3.9 KiB
Bash
Executable File
124 lines
3.9 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# Start a local Oracle 23ai Free container that mirrors what CI uses
|
|
# (`test-api-oracle`, `test-{python,typescript}-client-oracle`), bootstrap
|
|
# the HINDSIGHT_TEST user, and print the connection URL.
|
|
#
|
|
# Usage:
|
|
# ./scripts/dev/start-oracle.sh # start (idempotent) and print URL
|
|
# ./scripts/dev/start-oracle.sh --reset # drop and recreate the test user
|
|
#
|
|
# Stop with: ./scripts/dev/stop-oracle.sh
|
|
|
|
set -euo pipefail
|
|
|
|
CONTAINER_NAME="hindsight-oracle"
|
|
IMAGE="container-registry.oracle.com/database/free:latest"
|
|
PORT=1521
|
|
ORACLE_PWD="oracle"
|
|
TEST_USER="hindsight_test"
|
|
TEST_PASS="hindsight_test"
|
|
|
|
want_reset=0
|
|
for arg in "$@"; do
|
|
case "$arg" in
|
|
--reset) want_reset=1 ;;
|
|
*) echo "unknown arg: $arg" >&2; exit 2 ;;
|
|
esac
|
|
done
|
|
|
|
# 1) Ensure the container is running.
|
|
if [ -z "$(docker ps -q -f name="^${CONTAINER_NAME}$")" ]; then
|
|
if [ -n "$(docker ps -aq -f name="^${CONTAINER_NAME}$")" ]; then
|
|
echo "→ Removing stopped container ${CONTAINER_NAME}"
|
|
docker rm -f "${CONTAINER_NAME}" >/dev/null
|
|
fi
|
|
echo "→ Starting ${CONTAINER_NAME} (${IMAGE})"
|
|
docker run -d \
|
|
--name "${CONTAINER_NAME}" \
|
|
-p ${PORT}:1521 \
|
|
-e ORACLE_PWD="${ORACLE_PWD}" \
|
|
"${IMAGE}" >/dev/null
|
|
fi
|
|
|
|
# 2) Wait until SQL*Plus inside the container can SELECT 1 FROM DUAL.
|
|
# Same readiness probe CI uses for the service container's health-cmd.
|
|
echo "→ Waiting for FREEPDB1 to accept connections (~60-120s on a cold start) ..."
|
|
for i in $(seq 1 120); do
|
|
if docker exec "${CONTAINER_NAME}" \
|
|
bash -c "echo 'SELECT 1 FROM DUAL;' | sqlplus -s system/${ORACLE_PWD}@localhost:1521/FREEPDB1" \
|
|
>/dev/null 2>&1; then
|
|
echo " ready after ${i}s"
|
|
break
|
|
fi
|
|
if [ "$i" -eq 120 ]; then
|
|
echo "FREEPDB1 not ready after 120s — recent container logs:" >&2
|
|
docker logs --tail 60 "${CONTAINER_NAME}" >&2 || true
|
|
exit 1
|
|
fi
|
|
sleep 1
|
|
done
|
|
|
|
# 3) Bootstrap (or reset) the HINDSIGHT_TEST user. Mirrors the CI step.
|
|
if [ "$want_reset" = "1" ]; then
|
|
echo "→ --reset: dropping existing ${TEST_USER}"
|
|
docker exec -i "${CONTAINER_NAME}" \
|
|
sqlplus -s system/${ORACLE_PWD}@localhost:1521/FREEPDB1 <<SQL >/dev/null
|
|
DROP USER ${TEST_USER} CASCADE;
|
|
EXIT;
|
|
SQL
|
|
fi
|
|
|
|
echo "→ Ensuring ${TEST_USER} exists with ASSM tablespace (idempotent)"
|
|
# CREATE TABLESPACE / CREATE USER / GRANT statements are mirrored from the
|
|
# CI workflow steps (test-api-oracle, test-python-client-oracle, etc.).
|
|
# We tolerate "already exists" so the script can be re-run safely.
|
|
docker exec -i "${CONTAINER_NAME}" \
|
|
sqlplus -s system/${ORACLE_PWD}@localhost:1521/FREEPDB1 <<SQL >/dev/null
|
|
WHENEVER SQLERROR EXIT FAILURE
|
|
SET ECHO OFF
|
|
SET FEEDBACK OFF
|
|
|
|
DECLARE
|
|
e_already_exists EXCEPTION;
|
|
PRAGMA EXCEPTION_INIT(e_already_exists, -1543);
|
|
BEGIN
|
|
EXECUTE IMMEDIATE 'CREATE TABLESPACE hindsight_ts
|
|
DATAFILE ''hindsight_ts.dbf'' SIZE 200M AUTOEXTEND ON NEXT 50M
|
|
EXTENT MANAGEMENT LOCAL
|
|
SEGMENT SPACE MANAGEMENT AUTO';
|
|
EXCEPTION WHEN e_already_exists THEN NULL;
|
|
END;
|
|
/
|
|
|
|
DECLARE
|
|
e_user_exists EXCEPTION;
|
|
PRAGMA EXCEPTION_INIT(e_user_exists, -1920);
|
|
BEGIN
|
|
EXECUTE IMMEDIATE 'CREATE USER ${TEST_USER} IDENTIFIED BY ${TEST_PASS}
|
|
DEFAULT TABLESPACE hindsight_ts
|
|
TEMPORARY TABLESPACE temp
|
|
QUOTA UNLIMITED ON hindsight_ts';
|
|
EXCEPTION WHEN e_user_exists THEN NULL;
|
|
END;
|
|
/
|
|
|
|
GRANT CONNECT, RESOURCE, CREATE TABLE, CREATE SEQUENCE, CREATE VIEW, CREATE PROCEDURE TO ${TEST_USER};
|
|
GRANT CTXAPP TO ${TEST_USER};
|
|
EXIT;
|
|
SQL
|
|
|
|
URL="oracle+oracledb://${TEST_USER}:${TEST_PASS}@localhost:${PORT}/FREEPDB1"
|
|
cat <<EOF
|
|
|
|
✓ Oracle is ready.
|
|
|
|
Container : ${CONTAINER_NAME}
|
|
URL : ${URL}
|
|
|
|
export HINDSIGHT_API_DATABASE_BACKEND=oracle
|
|
export HINDSIGHT_API_DATABASE_URL='${URL}'
|
|
|
|
Migrations : uv run --directory hindsight-api-slim hindsight-admin run-db-migration
|
|
Stop : ./scripts/dev/stop-oracle.sh
|
|
EOF
|