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 chia farm summary aborting early if no local full node present #16387

Merged
merged 2 commits into from
Sep 25, 2023

Conversation

xearl4
Copy link
Contributor

@xearl4 xearl4 commented Sep 21, 2023

Before 2.0, even if no local full node was present, chia farm summary would still show useful information:

$ chia farm summary
Farming status: Not available
Local Harvester
   1 plots of size: 101.320 GiB on-disk, 101.400 GiBe (effective)
Plot count for all harvesters: 1
Total size of plots: 101.320 GiB, 101.400 GiBe (effective)
Estimated network space: Unknown
Expected time to win: Unknown
For details on farmed rewards and fees you should run 'chia start wallet' and 'chia wallet show'

However, since 2.0, that is no longer the case. chia farm summary simply aborts with an exception:

$ chia farm summary
Error: Connection error: ClientConnectorError: Cannot connect to host localhost:8555 ssl:<ssl.SSLContext object at 0x7fe0edd533c0> [Connect call failed ('127.0.0.1', 8555)]
Check if full node rpc is running at 8555
This is normal if full node is still starting up

This fixes this particular regression which is rather annoying for farmers farming to a remote node.

Fixes #16164. Related to issue #9615, but that issue is wider in scope.

@xearl4 xearl4 requested a review from a team as a code owner September 21, 2023 13:21
@paninaro paninaro requested a review from altendky September 21, 2023 16:30
@altendky altendky added the Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog label Sep 21, 2023
Copy link
Contributor

@altendky altendky left a comment

Choose a reason for hiding this comment

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

It would be nice to provide at least some information about the failure, in addition to the other information. At least for most errors. Maybe we could specifically handle ClientConnectorError in a quiet way and provide noisier output for all other exceptions? I'm ok with continuing on in both cases and providing the rest of the information we can provide.

@altendky
Copy link
Contributor

Also... Thank you!

@xearl4
Copy link
Contributor Author

xearl4 commented Sep 22, 2023

Yes, that's a good idea. Updated to silently handle ClientConnectorError and print the exception for all other exceptions.

@xearl4 xearl4 force-pushed the fix-chia-farm-summary branch 2 times, most recently from 499fcb2 to 5c1a3d6 Compare September 22, 2023 08:26
@xearl4 xearl4 force-pushed the fix-chia-farm-summary branch from 5c1a3d6 to 697b89c Compare September 22, 2023 12:13
@xearl4 xearl4 force-pushed the fix-chia-farm-summary branch from 697b89c to 0f6a317 Compare September 22, 2023 12:29
altendky
altendky previously approved these changes Sep 22, 2023
Copy link
Contributor

@altendky altendky left a comment

Choose a reason for hiding this comment

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

Looks good, pending CI. Thanks for the work and the contribution.

@altendky
Copy link
Contributor

Actually, sorry, could you rebase and retarget this to release/2.1.0? Or I can do that, if you've allowed me access to this branch via GitHub. Either way.

Before 2.0, even if no local full node was present, `chia farm summary`
would still show useful information:

	$ chia farm summary
	Farming status: Not available
	Local Harvester
	   1 plots of size: 101.320 GiB on-disk, 101.400 GiBe (effective)
	Plot count for all harvesters: 1
	Total size of plots: 101.320 GiB, 101.400 GiBe (effective)
	Estimated network space: Unknown
	Expected time to win: Unknown
	For details on farmed rewards and fees you should run 'chia start wallet' and 'chia wallet show'

However, since 2.0, that is no longer the case. `chia farm summary`
simply aborts with an exception:

	$ chia farm summary
	Error: Connection error: ClientConnectorError: Cannot connect to host localhost:8555 ssl:<ssl.SSLContext object at 0x7fe0edd533c0> [Connect call failed ('127.0.0.1', 8555)]
	Check if full node rpc is running at 8555
	This is normal if full node is still starting up

This fixes this particular regression which is rather annoying for
farmers farming to a remote node.

Fixes Chia-Network#16164. Related to issue Chia-Network#9615, but that issue is wider in scope.
With this change, only the exceptions when failing to connect to a local
full node and/or wallet are suppressed, as these cases are already
properly handled by adjusted command output. Any other exception is
printed to stderr.
@xearl4 xearl4 force-pushed the fix-chia-farm-summary branch from 0f6a317 to 9ffb0ce Compare September 23, 2023 14:25
@xearl4 xearl4 changed the base branch from main to release/2.1.0 September 23, 2023 14:25
@xearl4
Copy link
Contributor Author

xearl4 commented Sep 23, 2023

Actually, sorry, could you rebase and retarget this to release/2.1.0?

Sure, done. And thanks for your review!

@wallentx wallentx merged commit d293468 into Chia-Network:release/2.1.0 Sep 25, 2023
@xearl4 xearl4 deleted the fix-chia-farm-summary branch October 9, 2023 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] farmer-only connect to incorrect full node host
4 participants