search

waku-org / nwaku

Waku node and protocol.
Other
184 stars 47 forks source link

fix: add `meta` to sqlite migration scripts #2675

Closed richard-ramos closed 3 weeks ago

richard-ramos commented 3 weeks ago

Description

While running ./build/waku --store on latest master, i ran into the following error:

/home/richard/waku-org/nwaku/apps/wakunode2/wakunode2.nim(138) wakunode2
/home/richard/waku-org/nwaku/waku/factory/waku.nim(95) init
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(432) setupNode
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(113) setupProtocols
/home/richard/waku-org/nwaku/vendor/nim-chronos/chronos/internal/asyncfutures.nim(382) futureContinue
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(231) setupProtocols
/home/richard/waku-org/nwaku/waku/waku_archive/driver/builder.nim(24) new
/home/richard/waku-org/nwaku/vendor/nim-chronos/chronos/internal/asyncfutures.nim(382) futureContinue
/home/richard/waku-org/nwaku/waku/waku_archive/driver/builder.nim(81) new
/home/richard/waku-org/nwaku/waku/waku_archive/driver/sqlite_driver/sqlite_driver.nim(53) new
/home/richard/waku-org/nwaku/waku/waku_archive/driver/sqlite_driver/queries.nim(159) prepareInsertMessageStmt
/home/richard/waku-org/nwaku/vendor/nim-results/results.nim(376) raiseResultDefect
[[reraised from:
/home/richard/waku-org/nwaku/apps/wakunode2/wakunode2.nim(138) wakunode2
/home/richard/waku-org/nwaku/waku/factory/waku.nim(95) init
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(432) setupNode
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(113) setupProtocols
/home/richard/waku-org/nwaku/vendor/nim-chronos/chronos/internal/asyncfutures.nim(382) futureContinue
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(231) setupProtocols
/home/richard/waku-org/nwaku/waku/waku_archive/driver/builder.nim(24) new
/home/richard/waku-org/nwaku/vendor/nim-chronos/chronos/internal/asyncfutures.nim(382) futureContinue
/home/richard/waku-org/nwaku/waku/waku_archive/driver/builder.nim(125) new
]]
[[reraised from:
/home/richard/waku-org/nwaku/apps/wakunode2/wakunode2.nim(138) wakunode2
/home/richard/waku-org/nwaku/waku/factory/waku.nim(95) init
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(432) setupNode
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(113) setupProtocols
/home/richard/waku-org/nwaku/vendor/nim-chronos/chronos/internal/asyncfutures.nim(382) futureContinue
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(334) setupProtocols
]]
[[reraised from:
/home/richard/waku-org/nwaku/apps/wakunode2/wakunode2.nim(138) wakunode2
/home/richard/waku-org/nwaku/waku/factory/waku.nim(95) init
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(432) setupNode
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(113) setupProtocols
/home/richard/waku-org/nwaku/vendor/nim-chronos/chronos/internal/asyncfutures.nim(382) futureContinue
/home/richard/waku-org/nwaku/waku/factory/node_factory.nim(334) setupProtocols
]]
Error: unhandled exception: this is a valid statement: SQL logic error [ResultDefect]

When I browsed the db, and executed .schema message I saw that the problem was that the meta column did not exist.

Something curious tho, @Ivansete-status and @gabrielmer: when running sqlite, it seems that we use the .sql migration scripts, however we also have this createTableQuery proc: https://github.com/waku-org/nwaku/blob/38f8b08c7c818c2d3cb8460e972357e5a373c2b4/waku/waku_archive/driver/sqlite_driver/queries.nim#L86-L92 and createOldestMessageTimestampIndexQuery

Should we get rid of both of these so it's the migration scripts the ones responsible for updating the db schema for sqlite? based on the behavior before this fix, i'd say that it is highly likely that createTableQuery is not being used at all.

github-actions[bot] commented 3 weeks ago

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2675-rln-v2

Built from da062be995134efa97b2a889cb51f0660563fa76

github-actions[bot] commented 3 weeks ago

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2675-rln-v1

Built from da062be995134efa97b2a889cb51f0660563fa76

richard-ramos commented 3 weeks ago

@gabrielmer: Probably the table is created differently in the tests? Yeah! probably the tests use createTableQuery() while normal execution use the migration scripts!