Skip to content

Commit 0e564c6

Browse files
committed
fix(commands): address code-review findings in gbx:test:* --host mode
Follow-up to the --host commit; fixes six review findings, all in this session's new host-mode code. - PYSPARK_PYTHON not reaching Spark workers (gbx-test-python.sh): the `VAR=x eval "..."` command-prefix form does NOT export to the python grandchild, so workers fell back to system python3 (No module named pandas). New shared helper activate_host_python_env exports PYSPARK_PYTHON/PYSPARK_DRIVER_PYTHON and unsets PROJ_DATA/PROJ_LIB; all five host branches call it (kills the copy-pasted block too). - bench tests ran nowhere (coverage gap): the hand-maintained LIGHT_DIRS list omitted `bench`, so the heavy leg's conftest collect_ignore skipped it and the light leg didn't list it. Now the light-dir set is sourced from python/geobrix/test/conftest.py _LIGHT_TEST_DIRS (single source of truth) via host_light_test_dirs; the heavy leg runs plain `pytest test/` and lets that conftest's dependency-aware collect_ignore exclude the light dirs (no hand-rolled --ignore list to drift). - PROJ_DATA= empty-string vs unset: now unset (via the helper). - Unquoted --ignore/paths through eval: pytest is now invoked with a real args array (marker_args + path args), not an eval-interpolated string, so a repo path with spaces can't word-split. - Docs drift: five command .md Host-mode sections still described the abandoned single .venv-host / requirements-dev-container.txt model; updated to the two-venv (.venv-host-ci / .venv-host-pyrx) model. - Notebooks on host: the runner's nested `python -m venv` isolation fails on arca (no ensurepip). Host branch now sets GBX_NOTEBOOK_ISOLATED_ENV=0 and installs nbformat/nbconvert into .venv-host-pyrx, running notebooks directly in it. Re-verified in a clean shell (no pre-exported PYSPARK_PYTHON): unit heavy 146 passed, light leg green with bench now executing, function-info clean. Remaining failures are all pre-existing and unrelated to --host: bench fnspec references RST_FromFile.scala (absent on this branch), doc-test #51 /#52, /Volumes-hardcoded reads, and the gdal_examples import collision. Co-authored-by: Isaac
1 parent 4fdef3d commit 0e564c6

11 files changed

Lines changed: 105 additions & 69 deletions

scripts/commands/common.sh

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,40 @@ ensure_host_test_venv() {
267267
echo "${venv_dir}/bin"
268268
}
269269

270+
# Prepare the process environment so a host --host pytest run drives Spark + rasterio correctly.
271+
# Call with the venv bin dir (from ensure_host_test_venv) BEFORE launching pytest. Two exports the
272+
# forked Spark JVM's Python workers and rasterio need:
273+
# - PYSPARK_PYTHON / PYSPARK_DRIVER_PYTHON = the venv interpreter, so Spark workers use the venv
274+
# (pandas/pyarrow for Arrow UDFs live there, not in system python3). These MUST be real exports
275+
# in the current shell — a `VAR=x eval "..."` command-prefix does NOT propagate to the python
276+
# grandchild, so workers would silently fall back to system python3 (ModuleNotFound: pandas).
277+
# - unset PROJ_DATA / PROJ_LIB so the venv rasterio uses its own bundled proj.db (layout >=6);
278+
# the arca env points these at the older $HOME GDAL proj.db (layout 3), which rasterio rejects
279+
# (CRSError "... another PROJ installation"). unset (not empty-string) is required — an empty
280+
# PROJ_DATA is a search path of "", not a fallback to bundled data. The JVM GDAL sets PROJ_LIB
281+
# internally via SetConfigOption (the /usr/share/proj bridge), so the heavy tier is unaffected.
282+
# Usage: activate_host_python_env "$VENV_BIN"
283+
activate_host_python_env() {
284+
local venv_bin="$1"
285+
export PYSPARK_PYTHON="${venv_bin}/python"
286+
export PYSPARK_DRIVER_PYTHON="${venv_bin}/python"
287+
unset PROJ_DATA PROJ_LIB
288+
}
289+
290+
# The light-tier test dirs (single source of truth: python/geobrix/test/conftest.py _LIGHT_TEST_DIRS).
291+
# Their modules import light-only deps (rasterio/shapely/pandas/h3/…) at collection time, so they run
292+
# only in the pyrx venv; the ci venv's conftest collect_ignore skips them (rasterio absent). Echoed
293+
# space-separated. Falls back to a hardcoded list only if the conftest can't be parsed.
294+
host_light_test_dirs() {
295+
local conftest="${PROJECT_ROOT}/python/geobrix/test/conftest.py"
296+
local dirs
297+
dirs="$(awk '/_LIGHT_TEST_DIRS *= *\[/{f=1;next} f&&/\]/{f=0} f{gsub(/[",[:space:]]/,""); if($0!="") print}' "$conftest" 2>/dev/null | tr '\n' ' ')"
298+
if [ -z "${dirs// /}" ]; then
299+
dirs="bench ds pyrx pyvx pygx pmtiles_light stac vizx sample"
300+
fi
301+
echo "$dirs"
302+
}
303+
270304
# Run a command inside the isolated pyrx venv (host, no Docker).
271305
# Usage: run_in_pyrx_venv "<command string>"
272306
# Requires gbx:venv:sync to have been run (venv at $PROJECT_ROOT/.venv-pyrx).
@@ -291,4 +325,5 @@ export RED GREEN YELLOW BLUE CYAN NC DOCKER_MAVEN_ENV
291325
export -f check_docker resolve_log_path setup_log_file show_banner show_separator \
292326
print_report_link open_report generate_timestamp warn_if_jar_stale \
293327
print_banner print_separator setup_log run_in_pyrx_venv validate_set \
294-
require_host_gdal_env ensure_host_test_venv 2>/dev/null || true
328+
require_host_gdal_env ensure_host_test_venv activate_host_python_env \
329+
host_light_test_dirs 2>/dev/null || true

scripts/commands/gbx-test-docs.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ bash scripts/commands/gbx-test-docs.sh [OPTIONS]
3434

3535
## Host mode (arca, no Docker)
3636

37-
With `--host` the orchestrator runs on the host instead of `docker exec geobrix-dev`, forwarding `--host` to each child suite (python-docs, sql-docs, scala-docs). Prerequisites: `source ~/.local/geobrix-gdal-env.sh` (native GDAL + Java 17 + PYTHONPATH) and `uv` on PATH, with `PIP_INDEX_URL` pointing at the internal pip proxy. The venv-based child suites build `.venv-host` from `python/geobrix/requirements-dev-container.txt` on first run (the exact CI pins, minus native-source-only `pdal` which arca can't build); `--rebuild-venv` is forwarded to them. The scala-docs child runs `mvn` directly and needs only the sourced GDAL env. See the `geobrix-arca` plugin for the full setup.
37+
With `--host` the orchestrator runs on the host instead of `docker exec geobrix-dev`, forwarding `--host` to each child suite (python-docs, sql-docs, scala-docs). Prerequisites: `source ~/.local/geobrix-gdal-env.sh` (native GDAL + Java 17 + PYTHONPATH) and `uv` on PATH, with `PIP_INDEX_URL` pointing at the internal pip proxy. The venv-based child suites build a host test venv from the exact CI-pinned locks via `uv` on first run — `.venv-host-pyrx` from `python/geobrix/requirements-pyrx-ci.txt` (light-tier deps); neither CI lock contains `pdal` (source-only, unbuildable on arca); `--rebuild-venv` is forwarded to them. The scala-docs child runs `mvn` directly and needs only the sourced GDAL env. See the `geobrix-arca` plugin for the full setup.
3838

3939
**Sample data (default):** The command sets `GBX_SAMPLE_DATA_ROOT=/Volumes/main/default/test-data` inside the container so doc tests use the minimal bundle (host path `sample-data/Volumes/main/default/test-data`). This is required for running docs unit tests on remote/CI. Use `--no-sample-data-root` to leave it unset (e.g. to use a full bundle or your own env).
4040

scripts/commands/gbx-test-function-info.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ bash scripts/commands/gbx-test-function-info.sh [OPTIONS]
1111
## Options
1212

1313
- `--skip-generate` - Skip the generator; run only pytest in `docs/tests-function-info/`
14-
- `--host` - Run on the host (arca), not the Docker container. Requires `source ~/.local/geobrix-gdal-env.sh` first (provisioned by the `geobrix-arca` plugin); builds/reuses a `.venv-host` from the pinned lock and runs against a host-built JAR. See "Host mode" below.
14+
- `--host` - Run on the host (arca), not the Docker container. Requires `source ~/.local/geobrix-gdal-env.sh` first (provisioned by the `geobrix-arca` plugin); builds/reuses `.venv-host-pyrx` from the pinned CI lock and runs against a host-built JAR. See "Host mode" below.
1515
- `--rebuild-venv` - (with `--host`) force-rebuild the host test venv.
1616
- `--log <path>` - Write output to log file
1717
- `--help` - Display help
1818

1919
## Host mode (arca, no Docker)
2020

21-
With `--host` the command runs directly on the host instead of `docker exec geobrix-dev`. Prerequisites: `source ~/.local/geobrix-gdal-env.sh` (native GDAL + Java 17 + PYTHONPATH) and `uv` on PATH, with `PIP_INDEX_URL` pointing at the internal pip proxy. The pytest registers functions via the built JAR; no sample data is needed. The first run builds `.venv-host` from `python/geobrix/requirements-dev-container.txt` (the exact CI pins, minus native-source-only `pdal` which arca can't build). See the `geobrix-arca` plugin for the full setup.
21+
With `--host` the command runs directly on the host instead of `docker exec geobrix-dev`. Prerequisites: `source ~/.local/geobrix-gdal-env.sh` (native GDAL + Java 17 + PYTHONPATH) and `uv` on PATH, with `PIP_INDEX_URL` pointing at the internal pip proxy. The pytest registers functions via the built JAR; no sample data is needed. The first run builds a host test venv from the exact CI-pinned lock via `uv``.venv-host-pyrx` from `python/geobrix/requirements-pyrx-ci.txt` (the light-tier deps: rasterio/pandas/h3/vizx). Neither CI lock contains `pdal` (source-only, unbuildable on arca and not needed here). See the `geobrix-arca` plugin for the full setup.
2222

2323
## Default behavior (inside Docker)
2424

scripts/commands/gbx-test-function-info.sh

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,8 @@ if [ "$USE_HOST" = true ]; then
7171
# --- Host (arca) path: no Docker. The pytest registers functions via the built JAR. ---
7272
require_host_gdal_env || exit 1
7373
VENV_BIN=$(ensure_host_test_venv pyrx) || exit 1
74-
# Spark Python workers must use the venv interpreter (pandas/pyarrow for Arrow UDFs live there, not in system python3).
75-
export PYSPARK_PYTHON="$VENV_BIN/python"
76-
export PYSPARK_DRIVER_PYTHON="$VENV_BIN/python"
77-
# The venv rasterio bundles its own libproj/proj.db (layout >=6); the arca env points PROJ_DATA/PROJ_LIB
78-
# at the older $HOME GDAL proj.db (layout 3), which rasterio refuses. Unset them for the Python side so
79-
# rasterio uses its bundled data. The JVM GDAL sets PROJ_LIB internally via SetConfigOption (the
80-
# /usr/share/proj bridge), so this does not affect the heavy tier.
81-
unset PROJ_DATA PROJ_LIB
74+
# Wire Spark workers to the venv python and unset PROJ_DATA/PROJ_LIB (see common.sh).
75+
activate_host_python_env "$VENV_BIN"
8276
warn_if_jar_stale "$PROJECT_ROOT"
8377
unset JAVA_TOOL_OPTIONS
8478
export JUPYTER_PLATFORM_DIRS=1

scripts/commands/gbx-test-notebooks.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ bash scripts/commands/gbx-test-notebooks.sh [OPTIONS]
1414

1515
**Common**
1616

17-
- `--host` – Run on the host (arca), not the Docker container. Requires `source ~/.local/geobrix-gdal-env.sh` first (provisioned by the `geobrix-arca` plugin); builds/reuses a `.venv-host` from the pinned lock and runs against a host-built JAR. See "Host mode" below.
17+
- `--host` – Run on the host (arca), not the Docker container. Requires `source ~/.local/geobrix-gdal-env.sh` first (provisioned by the `geobrix-arca` plugin); builds/reuses `.venv-host-pyrx` from the pinned CI lock and runs against a host-built JAR. See "Host mode" below.
1818
- `--rebuild-venv` – (with `--host`) force-rebuild the host test venv.
1919
- `--log <path>` – Write output to log (filename → `test-logs/<name>`).
2020
- `--path <path>` – Limit scope: subdir (e.g. `sample-data`, `fixtures`), a single `.ipynb`, or a test file (e.g. `test_notebook_via_script.py`). With a `.py` path, runs **pytest** for that file instead of the cell-by-cell runner.
@@ -23,7 +23,9 @@ bash scripts/commands/gbx-test-notebooks.sh [OPTIONS]
2323

2424
## Host mode (arca, no Docker)
2525

26-
With `--host` the command runs directly on the host instead of `docker exec geobrix-dev`. Prerequisites: `source ~/.local/geobrix-gdal-env.sh` (native GDAL + Java 17 + PYTHONPATH) and `uv` on PATH, with `PIP_INDEX_URL` pointing at the internal pip proxy. The notebook runner executes from the host venv against the built JAR. The first run builds `.venv-host` from `python/geobrix/requirements-dev-container.txt` (the exact CI pins, minus native-source-only `pdal` which arca can't build). See the `geobrix-arca` plugin for the full setup.
26+
With `--host` the command runs directly on the host instead of `docker exec geobrix-dev`. Prerequisites: `source ~/.local/geobrix-gdal-env.sh` (native GDAL + Java 17 + PYTHONPATH) and `uv` on PATH, with `PIP_INDEX_URL` pointing at the internal pip proxy. The notebook runner executes from the host venv against the built JAR. The first run builds a host test venv from the exact CI-pinned lock via `uv``.venv-host-pyrx` from `python/geobrix/requirements-pyrx-ci.txt` (the light-tier deps: rasterio/pandas/h3/vizx), plus `nbformat`/`nbconvert` (needed by the runner, installed on demand). Neither CI lock contains `pdal` (source-only, unbuildable on arca and not needed here). Unlike the container, the host path runs the notebooks directly in `.venv-host-pyrx` (`GBX_NOTEBOOK_ISOLATED_ENV=0`) rather than a nested `python -m venv` (which fails on arca — no `ensurepip`/`python3-venv`).
27+
28+
**Note (bare host):** notebooks that write sample bundles to the literal `/Volumes` path (the data-download notebook) fail on a bare host with `Permission denied: '/Volumes'` — those need a real UC Volumes mount (`sudo ln -sfn "$PWD/sample-data/Volumes" /Volumes`) and, for Sentinel-2 fixtures, `pystac-client`/`planetary-computer`. The runner itself works on host; these are data/mount limitations. See the `geobrix-arca` plugin for the full setup.
2729

2830
**Read/write path behavior (absolute vs relative)**
2931

scripts/commands/gbx-test-notebooks.sh

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -133,17 +133,22 @@ if [ "$USE_HOST" = true ]; then
133133
# --- Host (arca) path: no Docker; run the notebook runner from .venv-host ---
134134
require_host_gdal_env || exit 1
135135
VENV_BIN=$(ensure_host_test_venv pyrx) || exit 1
136-
# Spark Python workers must use the venv interpreter (pandas/pyarrow for Arrow UDFs live there, not in system python3).
137-
export PYSPARK_PYTHON="$VENV_BIN/python"
138-
export PYSPARK_DRIVER_PYTHON="$VENV_BIN/python"
139-
# The venv rasterio bundles its own libproj/proj.db (layout >=6); the arca env points PROJ_DATA/PROJ_LIB
140-
# at the older $HOME GDAL proj.db (layout 3), which rasterio refuses. Unset them for the Python side so
141-
# rasterio uses its bundled data. The JVM GDAL sets PROJ_LIB internally via SetConfigOption (the
142-
# /usr/share/proj bridge), so this does not affect the heavy tier.
143-
unset PROJ_DATA PROJ_LIB
136+
# The notebook runner needs nbformat/nbconvert (not in the pyrx lock — the runner normally installs
137+
# them into its own nested venv). We run directly in .venv-host-pyrx (see below), so ensure they're
138+
# present here. Idempotent; uv no-ops when already installed.
139+
if ! "$VENV_BIN/python" -c "import nbformat, nbconvert" >/dev/null 2>&1; then
140+
echo -e "${CYAN}Installing nbformat/nbconvert into the host venv...${NC}"
141+
uv pip install --python "$VENV_BIN/python" -q nbformat nbconvert \
142+
|| { echo -e "${RED}❌ failed to install nbformat/nbconvert${NC}"; exit 1; }
143+
fi
144+
# Wire Spark workers to the venv python and unset PROJ_DATA/PROJ_LIB (see common.sh).
145+
activate_host_python_env "$VENV_BIN"
144146
unset JAVA_TOOL_OPTIONS
145147
export JUPYTER_PLATFORM_DIRS=1
146-
export GBX_NOTEBOOK_ISOLATED_ENV=1
148+
# Do NOT force the runner's nested-venv isolation on host: .venv-host-pyrx (plus nbformat/nbconvert
149+
# installed just above) is already the isolated env, and the runner's `python -m venv` nested-venv
150+
# path fails on arca (no ensurepip / python3-venv). Running directly in .venv-host-pyrx is the isolation.
151+
export GBX_NOTEBOOK_ISOLATED_ENV=0
147152
[ -n "$NOTEBOOK_VERBOSITY" ] && export GBX_NOTEBOOK_VERBOSITY="$NOTEBOOK_VERBOSITY"
148153
[ "$INCLUDE_INTEGRATION" = true ] && export GBX_NOTEBOOK_INCLUDE_INTEGRATION=1
149154
[ "$ALLOW_ABSOLUTE_READS" = true ] && export GBX_NOTEBOOK_ALLOW_ABSOLUTE_READS=1

scripts/commands/gbx-test-python-docs.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ bash scripts/commands/gbx-test-python-docs.sh [OPTIONS]
5858

5959
**Other**
6060

61-
- `--host` – Run on the host (arca), not the Docker container. Requires `source ~/.local/geobrix-gdal-env.sh` first (provisioned by the `geobrix-arca` plugin); builds/reuses a `.venv-host` from the pinned lock and a host-built JAR. See "Host mode" below.
61+
- `--host` – Run on the host (arca), not the Docker container. Requires `source ~/.local/geobrix-gdal-env.sh` first (provisioned by the `geobrix-arca` plugin); builds/reuses `.venv-host-pyrx` from the pinned CI lock and a host-built JAR. See "Host mode" below.
6262
- `--rebuild-venv` – (with `--host`) force-rebuild the host test venv.
6363
- `--log <path>` – Log file (filename → `test-logs/<name>`). Prefer timestamped names for tracking.
6464
- `--markers <markers>` – Pytest markers (e.g. `"not slow"`).
@@ -69,7 +69,7 @@ bash scripts/commands/gbx-test-python-docs.sh [OPTIONS]
6969

7070
## Host mode (arca, no Docker)
7171

72-
With `--host` the command runs directly on the host instead of `docker exec geobrix-dev`. Prerequisites: `source ~/.local/geobrix-gdal-env.sh` (native GDAL + Java 17 + PYTHONPATH) and `uv` on PATH, with `PIP_INDEX_URL` pointing at the internal pip proxy. The first run builds `.venv-host` from `python/geobrix/requirements-dev-container.txt` (the exact CI pins, minus native-source-only `pdal` which arca can't build). Sample data reads from the on-disk `sample-data/…/test-data` mirror via `GBX_SAMPLE_DATA_ROOT`. See the `geobrix-arca` plugin for the full setup.
72+
With `--host` the command runs directly on the host instead of `docker exec geobrix-dev`. Prerequisites: `source ~/.local/geobrix-gdal-env.sh` (native GDAL + Java 17 + PYTHONPATH) and `uv` on PATH, with `PIP_INDEX_URL` pointing at the internal pip proxy. The first run builds `.venv-host-pyrx` from `python/geobrix/requirements-pyrx-ci.txt` (the exact light-tier CI pins: rasterio/pandas/h3/vizx) via `uv`; neither CI lock contains `pdal` (source-only, unbuildable on arca and not needed here). Sample data reads from the on-disk `sample-data/…/test-data` mirror via `GBX_SAMPLE_DATA_ROOT`. See the `geobrix-arca` plugin for the full setup.
7373

7474
## Examples
7575

scripts/commands/gbx-test-python-docs.sh

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,10 @@ if [ "$USE_HOST" = true ]; then
142142
require_host_gdal_env || exit 1
143143
local_base="$PROJECT_ROOT/docs/tests/python"
144144
TEST_PATH="${local_base}/${REL_PATH}"
145-
IGNORE_API_ARG=""
146-
[ "$SKIP_SQL_TESTS" = true ] && IGNORE_API_ARG="--ignore=${local_base}/api"
145+
# Real args array (space-safe), not an eval-interpolated string, so a repo path with spaces
146+
# doesn't word-split the --ignore value.
147+
ignore_args=()
148+
[ "$SKIP_SQL_TESTS" = true ] && ignore_args=(--ignore="${local_base}/api")
147149

148150
echo -e "${CYAN}🎯 Test path: ${YELLOW}$TEST_PATH${NC} ${CYAN}(host)${NC}"
149151
[ "$SKIP_SQL_TESTS" = true ] && echo -e "${CYAN}🚫 Excluding api/ (use gbx:test:sql-docs or --suite api)${NC}"
@@ -152,14 +154,8 @@ if [ "$USE_HOST" = true ]; then
152154
echo ""
153155

154156
VENV_BIN=$(ensure_host_test_venv pyrx) || exit 1
155-
# Spark Python workers must use the venv interpreter (pandas/pyarrow for Arrow UDFs live there, not in system python3).
156-
export PYSPARK_PYTHON="$VENV_BIN/python"
157-
export PYSPARK_DRIVER_PYTHON="$VENV_BIN/python"
158-
# The venv rasterio bundles its own libproj/proj.db (layout >=6); the arca env points PROJ_DATA/PROJ_LIB
159-
# at the older $HOME GDAL proj.db (layout 3), which rasterio refuses. Unset them for the Python side so
160-
# rasterio uses its bundled data. The JVM GDAL sets PROJ_LIB internally via SetConfigOption (the
161-
# /usr/share/proj bridge), so this does not affect the heavy tier.
162-
unset PROJ_DATA PROJ_LIB
157+
# Wire Spark workers to the venv python and unset PROJ_DATA/PROJ_LIB (see common.sh).
158+
activate_host_python_env "$VENV_BIN"
163159

164160
if [ "$SKIP_BUILD" != true ]; then
165161
show_separator
@@ -175,7 +171,8 @@ if [ "$USE_HOST" = true ]; then
175171
show_separator
176172
echo -e "${CYAN}Running Python documentation tests (host)...${NC}"
177173
show_separator
178-
eval "\"$VENV_BIN/python\" -m pytest \"$TEST_PATH\" -v $MARKERS --tb=short --color=yes $IGNORE_API_ARG"
174+
marker_args=(); [ -n "$MARKERS" ] && eval "marker_args=($MARKERS)"
175+
"$VENV_BIN/python" -m pytest "$TEST_PATH" -v --tb=short --color=yes "${marker_args[@]}" "${ignore_args[@]}"
179176
EXIT_CODE=$?
180177
else
181178
# --- Docker path (unchanged) ---

0 commit comments

Comments
 (0)