cmd/geth: fixes db unavailability for chain commands #21415
Conversation
|
Some quick testing:
EDIT: all good! |
fjl
changed the title
cmd/geth/chaincmd.go
Outdated
|
|
||
| stack, _ := makeConfigNode(ctx) | ||
| defer stack.Close() | ||
| // Initialize a new chain for the running node to sync into |
There was a problem hiding this comment.
Code style nit, pls leave an empty line before a comment. Loosens the code up a bit :)
There was a problem hiding this comment.
Here again imho the original location was better. Mind you, you don't need an empty line there because } on itself is already loose enough :)
cmd/geth/chaincmd.go
Outdated
| stack, _ := makeConfigNode(ctx) | ||
| defer stack.Close() | ||
|
|
||
| // Open an initialise both full and light databases |
There was a problem hiding this comment.
Code style nit, pls leave an empty line before a comment. Loosens the code up a bit :)
There was a problem hiding this comment.
Also I don't see why this comment was moved really. I thin the original place is correct. The role of the comment was to explain in general what the next section does.
There was a problem hiding this comment.
Mind you, you don't need an empty line there because } on itself is already loose enough :)
There was a problem hiding this comment.
Ah yeah, sorry for shifting these around, I was originally commenting as to why I was making a makeConfigNode rather than makeFullNode but we shifted those comments to the functions themselves. Just moved it back, thanks for the feedback!
| @@ -425,7 +425,8 @@ func exportPreimages(ctx *cli.Context) error { | |||
| if len(ctx.Args()) < 1 { | |||
| utils.Fatalf("This command requires an argument.") | |||
| } | |||
| stack, _ := makeFullNode(ctx) | |||
|
|
|||
There was a problem hiding this comment.
@karalabe is this type of extra space fine or not necessary because of the } ?
There was a problem hiding this comment.
I'll leave it up to you :)
In general I only add an extra empty line if there's something super different coming up, but in that case I also add a comment :) All in all I don't want to micro manage people at this insanely low level, I just don't want comments to merge completely together with code :)
* chaincmd should make config nodes instead of full nodes * add documentation for using makeConfigNode instead of makeFullNode; * add documentation to functions * code style
Related to #21412, fixes database unavailability issues stemming from the eth service and trying to open node's DB simultaneously.