From e8602021b6a1e577371321791c19ac91f7e53dca Mon Sep 17 00:00:00 2001 From: Ivan Folgueira Bande <128452529+Ivansete-status@users.noreply.github.com> Date: Wed, 13 Sep 2023 12:45:55 +0200 Subject: [PATCH] chore(postgres): not loading the libpq library by default & better user 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. --- .github/workflows/ci.yml | 2 +- Makefile | 6 ++++- tests/all_tests_waku.nim | 3 ++- waku/waku_archive/archive.nim | 4 --- waku/waku_archive/driver/builder.nim | 40 ++++++++++++++++------------ 5 files changed, 31 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 43b73fd90..acb1473fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 diff --git a/Makefile b/Makefile index 3e62390e8..e083d3032 100644 --- a/Makefile +++ b/Makefile @@ -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 @@ -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 diff --git a/tests/all_tests_waku.nim b/tests/all_tests_waku.nim index 1284dd888..3b10bff18 100644 --- a/tests/all_tests_waku.nim +++ b/tests/all_tests_waku.nim @@ -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 diff --git a/waku/waku_archive/archive.nim b/waku/waku_archive/archive.nim index abdb6f94c..c8ffb7715 100644 --- a/waku/waku_archive/archive.nim +++ b/waku/waku_archive/archive.nim @@ -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, diff --git a/waku/waku_archive/driver/builder.nim b/waku/waku_archive/driver/builder.nim index 59bed30d8..f8a47afd7 100644 --- a/waku/waku_archive/driver/builder.nim +++ b/waku/waku_archive/driver/builder.nim @@ -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, @@ -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) + 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() + 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()) + 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) + 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"