-
Notifications
You must be signed in to change notification settings - Fork 502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
exp/services/ledgerexporter: Guide to installing and running ledger exporter #5355
Conversation
eed32fb
to
2f8b0ee
Compare
9613b99
to
53214a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the CLI section I like how you have added sections describing the usage and arguments for each command. However, I think the descriptions for the fill-gaps and append commands would be better left as is. There is a lot of detail in the command descriptions that @sreuland wrote in #5335 that appears to be edited away in this PR.
For example, the original description for append mode mentions the precondition for resumability:
This feature requires ledgers to be present on the remote data store for some (possibly empty) prefix of the requested range and then absent for the (possibly empty) remainder.
I think it's important we keep this info in the readme because that is the only place where we communicate the prerequisite
e81bcf9
to
cae76e0
Compare
You're right, It does leave out the following points for the
It might be more useful to provide guidelines on when to use each mode for the user but I am not entirely clear on them myself. Perhaps you or @sreuland could provide more insights into this? Thanks! |
cae76e0
to
b918d6f
Compare
Running However,
I mentioned an example above where the requested range is [2, 100] and we have already exported [2, 50]. In this example, the already exported prefix is [2, 50] and the absent remainder is [51, 100], thus the precondition is satisfied. Here are some other scenarios where the precondition is satisfied:
Here are some scenarios where the precondition is not satisfied:
Because the precondition is not satisfied in the 2 cases above, we cannot guarantee that
That part of the documentation is essentially stating that, given a requested ledger range
This property of
|
b918d6f
to
2dae44b
Compare
Oh, so the precondition is not necessarily for running in append mode; it's a precondition for guaranteeing an export without gaps. The phrase "This feature requires ledgers to be present" confused me into thinking that unless the condition is met, you cannot run in append mode. Now it makes sense. Thanks for explaining it in depth 🙏 I'll add the original description back. |
7d772d7
to
bbf04f8
Compare
PR Checklist
PR Structure
otherwise).
services/friendbot
, orall
ordoc
if the changes are broad or impact manypackages.
Thoroughness
.md
files, etc... affected by this change). Take a look in the
docs
folder for a given service,like this one.
Release planning
needed with deprecations, added features, breaking changes, and DB schema changes.
semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
This PR is part of hubble-382 which includes adding instructions on how to run the ledger exporter and explaining the command line options to the public docs. Currently, I am adding the installation and running guide to the README with the goal of keeping all necessary documentation for setting up and running the ledger exporter close to the code.
Additional docs, such as a developer guide is included in a separate PR. There will also be a guide for creating consumer apps using the data exported by the ledger exporter. Once all the documentation is in place, we can discuss and decide what information should be included in the public documentation and what can remain in the github repo.
Why
HUBBLE-382
Known limitations
N/A