lncli: add deletepayments command#5699
Merged
guggero merged 5 commits intolightningnetwork:masterfrom Sep 27, 2021
Merged
Conversation
|
Visit https://dashboard.github.orijtech.com?back=0&pr=5699&remote=true&repo=guggero%2Flnd to see benchmark details. |
carlaKC
approved these changes
Sep 7, 2021
Collaborator
carlaKC
left a comment
carlaKC
left a comment
There was a problem hiding this comment.
tACK for the last 4 commits, nice refactoring in this as well!
Only question I have here is whether we want to have an "are you sure + this might take a while" prompt for when we delete all payments?
guggero
force-pushed
the
lncli-deletepayment
branch
2 times, most recently
from
61c0a6f to
34a293a
Compare
Collaborator
Author
I think making it an explicit flag ( |
yyforyongyu
reviewed
Sep 8, 2021
Member
yyforyongyu
left a comment
yyforyongyu
left a comment
There was a problem hiding this comment.
LGMT, just a typo and pending a release note.
guggero
force-pushed
the
lncli-deletepayment
branch
from
34a293a to
a04cf31
Compare
guggero
force-pushed
the
lncli-deletepayment
branch
from
a04cf31 to
def463e
Compare
With this commit we move all commands that can be found within the "Payments" category into its own file called cmd_payments.go. The only exception are the Mission Control related commands which we are going to move to their own category and file in the next commit.
To make the "Payments" category a bit less overloaded and to move the Mission Control configuration commands away from the "root" category, we create the new "Mission Control" category that we move the commands into. We do this in a single commit so the next one where we move them into the same file can be a pure code move without any additional changes.
Having one file per sub command seems a bit too excessive and isn't really implemented for any of the other commands. So we just group the Mission Control commands into their own file for now.
guggero
force-pushed
the
lncli-deletepayment
branch
from
def463e to
48ca55f
Compare
Member
|
Lol I totally missed this when I made #5778 |
Roasbeef
requested changes
Sep 23, 2021
guggero
force-pushed
the
lncli-deletepayment
branch
from
48ca55f to
49ef29f
Compare
Member
|
Just needs the release notes, and this is g2g |
To give the CLI user the option to delete a single or multiple payments in one command, we expose the DeletePayment and DeleteAllPayments RPCs to the command line as well. Due to the similarity of the two RPCs we combine them into a single command with multiple flags. To make the command a bit more safe to run without arguments, we consciously switch the logic of the RPC flag "failed_payments_only" which is false by default into a "--include_non_failed" in the CLI which is false by thefault. So a user running the command without knowing what they are doing are only deleting failed payments by default, not all of the payments.
guggero
force-pushed
the
lncli-deletepayment
branch
from
2219812 to
57d203c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #5660.
To give the CLI user the option to delete a single or multiple payments
in one command, we expose the DeletePayment and DeleteAllPayments
RPCs to the command line as well. Due to the similarity of the two RPCs
we combine them into a single command with multiple flags.
To make the command a bit more safe to run without arguments, we
consciously switch the logic of the RPC flag "failed_payments_only"
which is false by default into a "--include_non_failed" in the CLI which
is false by thefault. So a user running the command without knowing what
they are doing are only deleting failed payments by default, not all of
the payments.
We also do some re-grouping and code movement to consolidate all payment
related commands into the same file.