search

apache / arrow-nanoarrow

Helpers for Arrow C Data & Arrow C Stream interfaces
https://arrow.apache.org/nanoarrow
Apache License 2.0
169 stars 35 forks source link

ci(python): Add Python to release verification with TEST_WITH_MEMCHECK=1 #446

Closed paleolimbot closed 5 months ago

paleolimbot commented 5 months ago

This can be run locally with:

export NANOARROW_PLATFORM=ubuntu
# On M1 you might need:
# export NANOARROW_ARCH=arm64
docker compose run --rm -e TEST_WITH_MEMCHECK=1 -e TEST_DEFAULT=0 -e TEST_PYTHON=1 -e VERBOSE=1 verify
jorisvandenbossche commented 5 months ago

Should we add Valgrind to normal CI instead of release verification? (is there something release specific about it?)

paleolimbot commented 5 months ago

is there something release specific about it?

Mostly we already had a TEST_WITH_MEMCHECK option for release verification (which also runs valgrind over the R and C tests), and so this is the most consistent place to put it (if a little non-standard compared to where another project might put it). It also lets you run this locally with docker compose run -e TEST_WITH_MEMCHECK=1 verify.

Should we add Valgrind to normal CI instead of release verification?

There are a very small subset of changes to the Python package that could introduce a memory leak (I had to introduce one on purpose to make sure that this worked!), so the weekly verification run is probably sufficient.