Skip to content

Conversation

@jholdstock
Copy link
Member

Checks were already in place for RPC version, but this was not sufficient to ensure that the binaries have actually been updated. New checks will now enforce the binary and RPC versions of the directly connected dcrd and dcrwallet, and also the dcrd which is backing the connected dcrwallets.

This is in anticipation of the file containing more generic version
functionality soon.
Checks were already in place for RPC version, but this was not
sufficient to ensure that the binaries have actually been updated. New
checks will now enforce the binary and RPC versions of the directly
connected dcrd and dcrwallet, and also the dcrd which is backing the
connected dcrwallets.
It was already enforced by version checks in the code but not called out
in the docs.
// version uses version RPC to retrieve the API version of dcrwallet.
func (c *WalletRPC) version() (*semver, error) {
var verMap map[string]dcrdtypes.VersionResult
err := c.Call(context.TODO(), "version", &verMap)
Copy link
Member

Choose a reason for hiding this comment

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

instead of introducing even more TODO, why not write this with context support and pass in TODO from the caller, if that is still needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I might just be naive here, but I believe the reality of the situation is that context just isn't needed in these RPC clients.

None of the calls are particularly long running, and in every case I think adding early interruption adds more complexity than its worth. The code is kept cleaner and data integrity is protected if we allow vspd to finish what it is doing before shutting down.

The one thing I might consider is adding a timeout to the context, but the underlying context would still be TODO or Background anyway.

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.

2 participants