-
Notifications
You must be signed in to change notification settings - Fork 678
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 a tool to write CryptoHash to DB #10766
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10766 +/- ##
==========================================
- Coverage 71.49% 71.47% -0.02%
==========================================
Files 758 759 +1
Lines 152284 152314 +30
Branches 152284 152314 +30
==========================================
+ Hits 108868 108871 +3
- Misses 38909 38933 +24
- Partials 4507 4510 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
pub(crate) struct WriteCryptoHashCommand { | ||
#[clap(long)] | ||
hash: near_primitives::hash::CryptoHash, | ||
#[clap(subcommand)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the fact that this is a subcommand is a bit weird to me, but I guess it's okay. Why not just another long arg, like --column
, and then --key
could also be another long arg that accepts arbitrary bytes. And if you want to write to the STATE_SNAPSHOT_KEY
you could either spell it out in the argument to --key
or there could be yet another long arg, --state-snapshot-key
, that's a shorthand for that, that you could give in place of --key
. just a thought though
column: ColumnSelector, | ||
} | ||
|
||
impl WriteCryptoHashCommand { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this command is quite dangerous. Let's say that I run it and then it's in my bash history. And then I'm scrolling through my bash history to find some unrelated thing and I accidentally re-run the previous invocation of this command. This would be quite sad, and quite a bit sadder than accidentally running neard run
. Is it worth adding a prompt that the user needs to explicitly agree to (without a -f
flag or something)?
@marcelo-gonzalez We should also add this to 1.38.0. It will not break anything, but improve mitigation experience if we are to fail snapshot creation again.
The tool in 1.37.0-fix was called like this
Now it should be called like this