Skip to content
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

fix:cli:Continue instead of return error if no valid value is filled #8131

Merged
merged 6 commits into from
Feb 20, 2022

Conversation

FlorianRuen
Copy link
Contributor

Related Issues

Fixes #8100

Proposed Changes

If no valid value are filled in "Deals to make (1)", display an error message and continue to ask
(instead of return error to make the CLI crash)

Additional Info

Maybe we can force a default value to 1, instead of asking in loop for a valid value ?

Bug fixed : if hitting return instead of filling with a valid value, the CLI crashed (line 796)
Change return err by continue to ask a valid value again
Bug fixed : if hitting return instead of filling with a valid value, the CLI crashed (line 796)
Change return err by continue to ask a valid value again
@FlorianRuen FlorianRuen requested a review from a team as a code owner February 17, 2022 17:39
@FlorianRuen FlorianRuen changed the title _fix_:cli:Continue instead of return error if no valid value is filled fix:cli:Continue instead of return error if no valid value is filled Feb 17, 2022
Commit to fix lint reported errors during tests on CircleCI
(remove blank lines x2)
Commit to fix lint reported errors during tests on CircleCI
(remove trailling whitespace)
@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #8131 (623272a) into master (93bc3af) will decrease coverage by 0.05%.
The diff coverage is 40.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8131      +/-   ##
==========================================
- Coverage   39.39%   39.33%   -0.06%     
==========================================
  Files         666      666              
  Lines       72466    72469       +3     
==========================================
- Hits        28546    28505      -41     
- Misses      38995    39027      +32     
- Partials     4925     4937      +12     
Impacted Files Coverage Δ
cli/client.go 22.21% <40.00%> (+0.09%) ⬆️
markets/loggers/loggers.go 88.88% <0.00%> (-11.12%) ⬇️
chain/actors/builtin/miner/diff.go 48.52% <0.00%> (-10.30%) ⬇️
chain/exchange/peer_tracker.go 66.66% <0.00%> (-4.31%) ⬇️
miner/warmup.go 70.83% <0.00%> (-4.17%) ⬇️
storage/wdpost_sched.go 77.45% <0.00%> (-3.93%) ⬇️
storage/miner.go 69.31% <0.00%> (-2.28%) ⬇️
blockstore/buffered.go 34.44% <0.00%> (-2.23%) ⬇️
blockstore/autobatch.go 56.30% <0.00%> (-1.69%) ⬇️
storage/wdpost_changehandler.go 97.64% <0.00%> (-0.95%) ⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93bc3af...623272a. Read the comment docs.

Clear the list of miner addresses and successfull get-asked before asking for new ones
Previous behavior : if get-ask failed for miner(s), the faulty miner will be retried each time,
so you have to stop the command and start again to change this faulty miner (instead of removing from the list on the new attempt)
@magik6k
Copy link
Contributor

magik6k commented Feb 20, 2022

Maybe we can force a default value to 1, instead of asking in loop for a valid value ?

For deal count, yeah, that makes sense, though I'd make that 3, at least, so that there is at least some default redundancy.

Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@magik6k magik6k merged commit 214c32d into filecoin-project:master Feb 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cli lotus client deal input validation crash
2 participants