feat(wakunode2): simplify wakunode2 config and decouple peer persistence #1300
Conversation
LNSD
changed the title
Jenkins BuildsClick to see older builds (6)
|
LNSD
force-pushed
the
feat-wakunode2-storesetup
branch
from
37dffa7 to
b533a01
Compare
| let store = StoreQueueRef.new(storeCapacity) | ||
| return ok(store) | ||
|
|
||
| proc setupMessageStoreRetentionPolicy(retentionPolicy: string): SetupResult[Option[MessageRetentionPolicy]] = |
There was a problem hiding this comment.
Should we use an optional enum for the retentionPolicy argument since we have only 2, or no retention policy at all?
There was a problem hiding this comment.
Can you elaborate on that? Where are you suggesting using the enum?
- Return type?
- Input argument?
- Config?
There was a problem hiding this comment.
in the input, sorry. retentionPolicy: string
| ok() | ||
|
|
||
| proc startMetricsServer(node: WakuNode, address: ValidIpAddress, port: uint16, portsShift: uint16): SetupResult[void] = | ||
| startMetricsServer(address, Port(port + portsShift)) |
There was a problem hiding this comment.
Well... the compiler is not complaining, so I don't think we should add a try/catch there.
But I agree that the error management in the metrics, JSON-RPC, and rest servers setup procedure has to be improved.
Co-authored-by: Aaryamann Challani <[email protected]>
Co-authored-by: Aaryamann Challani <[email protected]>
@jakubgs In this PR, we are dropping the deprecated options. So in the next release, v0.13.0, the store configuration will change. Do you have any concerns before merging the PR? |
|
Yes, we'll need to coordinate the deployment to fleets or shit will break. I'm in Istanbul next week and off most of week after that, so if we want this done it would probably have to be tomorrow. Or on the 9th possibly. |
|
No need to rush. AFAIK, we are not planning to do any rollout for now. We can wait until you are back. Probably it is a better idea because I am planning to add support for configuring via environment variables. So we do not re-work things twice. |
This is the second part of #1293, and resolves #1103:
--storenodefromresumeStoreNodewakunode2storage setup proceduresSqliteDatabasecompatible withdbUrlpathwakunode2/config.nimNOTE: Some extra work is necessary to improve the message store options (e.g., implement the unimplemented methods in the in-memory store, decouple the message store from the waku store protocol, etc.). This will be done in future PRs.