Feat: Lotus Daemon CLI: Auto remove existing chain if importing chain file or snapshot #11277
Conversation
|
please do follow the pr template and fill the relevant part in! |
cmd/lotus/daemon.go
Outdated
| willImportChain = true | ||
| } | ||
|
|
||
| if cctx.Bool("remove-existing-chain") || willImportChain { |
There was a problem hiding this comment.
should we specify the behaviour expected if remove-existing-chain is set to be false yet willImportChain is true? warning & remove, or error out? or follow remove-existing-chainto not remove and continue with import?
There was a problem hiding this comment.
Good point, maybe we should just error out. Otherwise we are somehow creating undefined behaviors?
There was a problem hiding this comment.
@mb1896 What I would suggest is:
- if
remove-existing-chain, just remove the chain - if it's not specified, prompt the user with a y/n question asking whether they want the chain to be deleted -- if yes, do so, if not proceed without doing so
There was a problem hiding this comment.
Thanks for the comments! Fixed
There was a problem hiding this comment.
@arajasek Please take another look.
mb1896
changed the title
mb1896
force-pushed
the
0919_jie_flip_flags
branch
from
1167961
to
bdffac3
Compare
CHANGELOG.md
Outdated
| @@ -34,6 +34,7 @@ This feature release requires a **minimum Go version of v1.19.12 or higher to su | |||
| - feat: Lotus Gateway: add MpoolPending, ChainGetBlock and MinerGetBaseInfo ([filecoin-project/lotus#10929](https://github.com/filecoin-project/lotus/pull/10929)) | |||
|
|
|||
| ## Improvements | |||
| - chore: Auto remove local chain data when importing chain file or snapshot ([filecoin-project/lotus#11277](https://github.com/filecoin-project/lotus/pull/11277)) | |||
There was a problem hiding this comment.
Thanks for including this! It needs to be moved up to the UNRELEASED section, though
| if willImportChain && !willRemoveChain { | ||
| // Confirm with the user about the intention to remove chain data. | ||
| reader := bufio.NewReader(os.Stdin) | ||
| fmt.Print("Importing chain or snapshot will by default delete existing local chain data. Do you want to proceed and delete? (yes/no): ") |
There was a problem hiding this comment.
| fmt.Print("Importing chain or snapshot will by default delete existing local chain data. Do you want to proceed and delete? (yes/no): ") | |
| fmt.Print("Do you want to delete existing local chain data as part of the import process? (yes/no): ") |
There was a problem hiding this comment.
(just a bit more clear, there isn't really a default since we're forcing users to specify)
There was a problem hiding this comment.
You are right, however we couldn't differentiate between
- user not specifying a flag's value
- user specifically specifying a flag to false
That's why we have to ask for the user's input...
| } | ||
| userInput = strings.ToLower(strings.TrimSpace(userInput)) | ||
|
|
||
| if userInput == "yes" { |
There was a problem hiding this comment.
Probably want to handle case-sensitivity here
There was a problem hiding this comment.
The ToLower() function handled it.
mb1896
force-pushed
the
0919_jie_flip_flags
branch
from
4dce700
to
49044b4
Compare
Related Issues
Based on the discussion https://filecoinproject.slack.com/archives/CP50PPW2X/p1695108301930859
Proposed Changes
When user wants to import chain file or snapshot, we will by default remove the local chain data, unless the user says otherwise.
Additional Info
None.
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>fix: mempool: Introduce a cache for valid signaturesPR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, testarea, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps