[sql-29]: firewalldb: implement SQL version of RulesDB (kvstores) #1030
Conversation
ellemouton
added
the
no-changelog
ellemouton
force-pushed
the
sql29
branch
7 times, most recently
from
84ce3f0 to
c3beafe
Compare
ellemouton
requested review from
ViktorTigerstrom and
bitromortac
and removed request for
ViktorTigerstrom
terminal.go
Outdated
| @@ -238,6 +238,7 @@ func New() *LightningTerminal { | |||
| type stores struct { | |||
| accounts accounts.Store | |||
| sessions session.Store | |||
| rules firewalldb.RulesDB | |||
|
|
|||
| firewall *firewalldb.DB | |||
There was a problem hiding this comment.
there is some redundancy between rules and firewall, is it possible to remove one of them, or how will that look in the end? will we have a single firewall db here or the individual RulesDB/PrivacyDB?
There was a problem hiding this comment.
it's not redundant in the dev build case.
THe DB is a subsystem that needs to be started/stopped. The RulesDB is an interface that needs to be satisfied.
Currently, in the dev build, the RulesDB is either backed by the firewallDB or by the sql store. In the prod build, yes it is redundant.
eventually: we only will need the firewalldb.DB & can just pass that around.
There was a problem hiding this comment.
right, I think it's still possible and rules it not needed, unless I misunderstood you, see the patch
changes.txt
There was a problem hiding this comment.
ah ok yes indeed - you're totally right. Thanks for the call out 🙏
There was a problem hiding this comment.
thank you for the update!
There was a problem hiding this comment.
ah, the diff needed one more thing i think. fixed now i hope 🤞
Here we add the SQLDB struct which will later on be used to implement the SQL versions of the various firewall DBs. We also add the sqlExecutor here which is the SQL version of `DBExecutor`. This will be used for both the KVstores DB and privacy mapper DBs.
So that we can extract the same BaseDB in other packages and use it for initialising other SQL stores during tests.
terminal.go
Outdated
| @@ -238,6 +238,7 @@ func New() *LightningTerminal { | |||
| type stores struct { | |||
| accounts accounts.Store | |||
| sessions session.Store | |||
| rules firewalldb.RulesDB | |||
|
|
|||
| firewall *firewalldb.DB | |||
There was a problem hiding this comment.
it's not redundant in the dev build case.
THe DB is a subsystem that needs to be started/stopped. The RulesDB is an interface that needs to be satisfied.
Currently, in the dev build, the RulesDB is either backed by the firewallDB or by the sql store. In the prod build, yes it is redundant.
eventually: we only will need the firewalldb.DB & can just pass that around.
|
note: will fix linter after next review round |
In this commit, we let the SQLDB implement the RulesDB interface. Unit tests are updated such that they can be run against sqlite and postgres DBs using the SQLDB.
terminal.go
Outdated
| @@ -238,6 +238,7 @@ func New() *LightningTerminal { | |||
| type stores struct { | |||
| accounts accounts.Store | |||
| sessions session.Store | |||
| rules firewalldb.RulesDB | |||
|
|
|||
| firewall *firewalldb.DB | |||
There was a problem hiding this comment.
right, I think it's still possible and rules it not needed, unless I misunderstood you, see the patch
changes.txt
Plug the new SQL version of the RulesDB into LiT for `dev` flag builds.
This PR adds the SQL version of the RulesDb
part of #917