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

add explicit error msg if repo dir does not exist #6909

Merged
merged 1 commit into from
Aug 1, 2021

Conversation

nonsense
Copy link
Member

@nonsense nonsense commented Jul 28, 2021

This PR is adding an explicit error in case someone tries to run CLI pointed at incorrect repo location. We had a lot of confusion with this yesterday, given that miners are now running multiple miner instances on one host.

# node not started
➜  lotus git:(nonsense/better-error-on-wrong-repo-location) ./lotus-miner info
ERROR: could not get API info: could not get api endpoint: API not running (no endpoint)

# repo directory wrong / configuration problem
➜  lotus git:(nonsense/better-error-on-wrong-repo-location) LOTUS_MINER_PATH=~/smthwrong ./lotus-miner info
ERROR: could not get API info: repo directory does not exist. Make sure your configuration is correct.

@nonsense nonsense requested review from raulk and magik6k July 28, 2021 14:58
@nonsense nonsense requested a review from a team as a code owner July 28, 2021 14:58
@nonsense nonsense force-pushed the nonsense/better-error-on-wrong-repo-location branch 2 times, most recently from 6b83d77 to 27eebba Compare July 28, 2021 15:07
cli/util/api.go Outdated
}

if !exists {
return APIInfo{}, errors.New("repo directory does not exist. Make sure your configuration is correct")
Copy link
Contributor

Choose a reason for hiding this comment

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

What do we print if you run lotus-miner init .. without a full-node repo? (just want to make sure we won't make that message really confusing)

Copy link
Member Author

@nonsense nonsense Jul 29, 2021

Choose a reason for hiding this comment

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

# repo dir doesn't exist
➜  LOTUS_PATH=~/.lotussss LOTUS_MINER_PATH=~/.restoredrepo ./lotus-miner init service --type=markets --api-sealer=$APISEALER --api-sector-index=$APISEALER --config=~/.lotusmarket/config.toml ~/.lotusbackup/backupfile
2021-07-29T14:54:08.146+0300    INFO    main    lotus-miner/init_service.go:54  Initializing lotus miner service
2021-07-29T14:54:08.147+0300    INFO    main    lotus-miner/init_restore.go:102 Trying to connect to full node RPC
ERROR: could not get API info: repo directory does not exist. Make sure your configuration is correct

# no LOTUS_PATH, so ~/.lotus is inferred and it just works
➜  LOTUS_MINER_PATH=~/.restoredrepo ./lotus-miner init service --type=markets --api-sealer=$APISEALER --api-sector-index=$APISEALER --config=~/.lotusmarket/config.toml ~/.lotusbackup/backupfile

# no LOTUS_PATH, but this time repo exists (so config is correct), but node is not running
➜  LOTUS_MINER_PATH=~/.restoredrepo ./lotus-miner init service --type=markets --api-sealer=$APISEALER --api-sector-index=$APISEALER --config=~/.lotusmarket/config.toml ~/.lotusbackup/backupfile
2021-07-29T14:57:18.953+0300    INFO    main    lotus-miner/init_service.go:54  Initializing lotus miner service
2021-07-29T14:57:18.953+0300    INFO    main    lotus-miner/init_restore.go:102 Trying to connect to full node RPC
ERROR: could not get API info: could not get api endpoint: API not running (no endpoint)

@jacobheun jacobheun added release/backport team/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs labels Jul 28, 2021
@raulk raulk added the P2 P2: Should be resolved label Jul 29, 2021
@jennijuju jennijuju force-pushed the nonsense/better-error-on-wrong-repo-location branch 2 times, most recently from ca38ec9 to 5b1fdb9 Compare August 1, 2021 04:19
@jennijuju jennijuju force-pushed the nonsense/better-error-on-wrong-repo-location branch from 5b1fdb9 to fa7e52d Compare August 1, 2021 04:23
@jennijuju jennijuju mentioned this pull request Aug 1, 2021
1 task
@jennijuju
Copy link
Member

Rebased and I’m gonna merge it and backport it in the v1.11.1 release branch so it’s in v1.11.1-rc2. Please open new prs if we need further improvements on it.

@jennijuju jennijuju merged commit 9e707e1 into master Aug 1, 2021
@jennijuju jennijuju deleted the nonsense/better-error-on-wrong-repo-location branch August 1, 2021 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 P2: Should be resolved team/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants