Skip to content

Commit

Permalink
chore(postgres): not loading the libpq library by default & better us…
Browse files Browse the repository at this point in the history
…er feedback (#2028)

* removing implicit dependency with libpq if postgres is not being used.
* We only run the postgres tests when explicitly willing to, i.e. make
POSTGRES=1 test. The reason is that the postgres tests expect a
database instance to be running locally.
  • Loading branch information
Ivansete-status authored Sep 13, 2023
1 parent 563b2b2 commit e860202
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 27 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ jobs:
sudo docker run --rm -d -e POSTGRES_PASSWORD=test123 -p 5432:5432 postgres:9.6-alpine
fi
make V=1 LOG_LEVEL=DEBUG QUICK_AND_DIRTY_COMPILER=1 test testwakunode2
make V=1 LOG_LEVEL=DEBUG QUICK_AND_DIRTY_COMPILER=1 POSTGRES=1 test testwakunode2
build-docker-image:
needs: changes
Expand Down
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ ifneq ($(USE_LIBBACKTRACE), 0)
deps: | libbacktrace
endif

ifeq ($(POSTGRES), 1)
NIM_PARAMS := $(NIM_PARAMS) -d:postgres -d:nimDebugDlOpen
endif

clean: | clean-libbacktrace


Expand Down Expand Up @@ -218,7 +222,7 @@ docs: | build deps
#####################
# -d:insecure - Necessary to enable Prometheus HTTP endpoint for metrics
# -d:chronicles_colors:none - Necessary to disable colors in logs for Docker
DOCKER_IMAGE_NIMFLAGS ?= -d:chronicles_colors:none -d:insecure
DOCKER_IMAGE_NIMFLAGS ?= -d:chronicles_colors:none -d:insecure -d:postgres
DOCKER_IMAGE_NIMFLAGS := $(DOCKER_IMAGE_NIMFLAGS) $(HEAPTRACK_PARAMS)

# build a docker image for the fleet
Expand Down
3 changes: 2 additions & 1 deletion tests/all_tests_waku.nim
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ import
./waku_archive/test_waku_archive

const os* {.strdefine.} = ""
when os == "Linux":
when os == "Linux" and
# GitHub only supports container actions on Linux
# and we need to start a postgress database in a docker container
defined(postgres):
import
./waku_archive/test_driver_postgres_query,
./waku_archive/test_driver_postgres
Expand Down
4 changes: 0 additions & 4 deletions waku/waku_archive/archive.nim
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ import
../common/databases/dburl,
../common/databases/db_sqlite,
./driver,
./driver/queue_driver,
./driver/sqlite_driver,
./driver/sqlite_driver/migrations as archive_driver_sqlite_migrations,
./driver/postgres_driver/postgres_driver,
./retention_policy,
./retention_policy/retention_policy_capacity,
./retention_policy/retention_policy_time,
Expand Down
46 changes: 26 additions & 20 deletions waku/waku_archive/driver/builder.nim
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ import
../../common/databases/db_sqlite,
./sqlite_driver,
./sqlite_driver/migrations as archive_driver_sqlite_migrations,
./queue_driver,
./postgres_driver
./queue_driver

export
sqlite_driver,
queue_driver,
postgres_driver
queue_driver

when defined(postgres):
import ./postgres_driver ## This import adds dependency with an external libpq library
export postgres_driver

proc new*(T: type ArchiveDriver,
url: string,
Expand Down Expand Up @@ -78,22 +80,26 @@ proc new*(T: type ArchiveDriver,
return ok(res.get())

of "postgres":
const MaxNumConns = 5 #TODO: we may need to set that from app args (maybe?)
let res = PostgresDriver.new(url, MaxNumConns, onErrAction)
if res.isErr():
return err("failed to init postgres archive driver: " & res.error)

let driver = res.get()

try:
# The table should exist beforehand.
let newTableRes = waitFor driver.createMessageTable()
if newTableRes.isErr():
return err("error creating table: " & newTableRes.error)
except CatchableError:
return err("exception creating table: " & getCurrentExceptionMsg())

return ok(driver)
when defined(postgres):
const MaxNumConns = 5 #TODO: we may need to set that from app args (maybe?)
let res = PostgresDriver.new(url, MaxNumConns, onErrAction)
if res.isErr():
return err("failed to init postgres archive driver: " & res.error)

let driver = res.get()

try:
# The table should exist beforehand.
let newTableRes = waitFor driver.createMessageTable()
if newTableRes.isErr():
return err("error creating table: " & newTableRes.error)
except CatchableError:
return err("exception creating table: " & getCurrentExceptionMsg())

return ok(driver)

else:
return err("Postgres has been configured but not been compiled. Check compiler definitions.")

else:
debug "setting up in-memory waku archive driver"
Expand Down

0 comments on commit e860202

Please sign in to comment.