Modularise WalletDB and Manifest - #6223
Conversation
af0b679 to
4e554cc
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #6223 +/- ##
=========================================
- Coverage 79.9% 79.9% -0.0%
=========================================
Files 840 840
Lines 65514 65522 +8
Branches 7269 7249 -20
=========================================
+ Hits 52360 52362 +2
- Misses 13154 13160 +6
🚀 New features to boost your workflow:
|
7c5faca to
09b52e4
Compare
c9dbea1 to
8dc9483
Compare
572f5b2 to
650dc4f
Compare
godexsoft
left a comment
There was a problem hiding this comment.
Leaving some suggestions.
| isStopping() const = 0; | ||
|
|
||
| virtual beast::Journal | ||
| journal(std::string const& name) = 0; |
There was a problem hiding this comment.
this should be getJournal and getLog to match other service locator accessors (maybe in another pr)
There was a problem hiding this comment.
This is going to be a big PR https://github.com/XRPLF/rippled/pull/new/a1q123456/rename-xxx-to-getXxx
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
0dd8b72 to
b3f0361
Compare
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
b3f0361 to
c0bbbb2
Compare
| } | ||
| else | ||
| { | ||
| Throw<std::runtime_error>("Invalid journal_mode value: " + journal_mode); |
There was a problem hiding this comment.
Do we really want to terminate if journal_mode is invalid? Should it not revert to a default value?
There was a problem hiding this comment.
Same, I don't think we should correct anything and try to hide the problem here.
| // #synchronous Valid values : off, normal, full, extra | ||
| if (set(synchronous, "synchronous", sqlite) && !safety_level.empty()) | ||
| { | ||
| Throw<std::runtime_error>( |
There was a problem hiding this comment.
Shouldn't we log these errors instead and continue with a default correct combination of values?
There was a problem hiding this comment.
I don't think so. If the user input is wrong, we definitely want the user to correct it. The user may be surprised otherwise.
pratikmankawde
left a comment
There was a problem hiding this comment.
Left some comments.
godexsoft
left a comment
There was a problem hiding this comment.
LGTM for a refactoring PR 👍 ![]()
Please create issues for everything that has been noted by @pratikmankawde and myself.
|
RIPD-4880 Ticket created. |
pratikmankawde
left a comment
There was a problem hiding this comment.
Approved with one suggestion about cleaning up the lambda in src/xrpld/core/detail/Config.cpp::setup_Databasecon(..)
…se-wallet-db-and-manifest
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
This change modularizes the `WalletDB` and `Manifest`. Note that the wallet db has nothing to do with account wallets and it stores node configuration, which is why it depends on the manifest code.
High Level Overview of Change
This PR modularises the walletdb and manifest. Please be aware that the wallet db actually has nothing to do with account wallets and it stores node configuration, which is why it depends on
Manifest.Context of Change
To help modularisation, the wallet db has to be modularised.
This PR is part of the modularisation of the transactors. See all PRs here:
#6222
#6223
#6224
#6225
#6226
#6227
#6228
Type of Change
.gitignore, formatting, dropping support for older tooling)