-
Notifications
You must be signed in to change notification settings - Fork 356
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
Progress bars for key generation #144
Conversation
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.
Nice feature! 🦏
Just some nitpicking.
|
||
def export_keystores(self, password: str, folder: str) -> List[str]: | ||
return [credential.save_signing_keystore(password=password, folder=folder) for credential in self.credentials] | ||
with click.progressbar(self.credentials, label='Creating your keystores:\t', |
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.
nitpicking: should we keep using "(s)"?
with click.progressbar(self.credentials, label='Creating your keystores:\t', | |
with click.progressbar(self.credentials, label='Creating your keystore(s):\t', |
|
||
def export_deposit_data_json(self, folder: str) -> str: | ||
deposit_data = [cred.deposit_datum_dict for cred in self.credentials] | ||
with click.progressbar(self.credentials, label='Creating your depositdata:\t', |
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.
with click.progressbar(self.credentials, label='Creating your depositdata:\t', | |
with click.progressbar(self.credentials, label='Creating your deposit data:\t', |
filefolder = os.path.join(folder, 'deposit_data-%i.json' % time.time()) | ||
with open(filefolder, 'w') as f: | ||
json.dump(deposit_data, f, default=lambda x: x.hex()) | ||
return filefolder | ||
|
||
def verify_keystores(self, keystore_filefolders: List[str], password: str) -> bool: | ||
return all(credential.verify_keystore(keystore_filefolder=filefolder, password=password) | ||
for credential, filefolder in zip(self.credentials, keystore_filefolders)) | ||
with click.progressbar(zip(self.credentials, keystore_filefolders), label='Verifying your keystores:\t', |
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.
with click.progressbar(zip(self.credentials, keystore_filefolders), label='Verifying your keystores:\t', | |
with click.progressbar(zip(self.credentials, keystore_filefolders), label='Verifying your keystore(s):\t', |
Creating your keys: [####################################] <N>/<N> | ||
Creating your keystores: [####################################] <N>/<N> | ||
Creating your depositdata: [####################################] <N>/<N> | ||
Verifying your keystores: [####################################] <N>/<N> | ||
Verifying your deposits: [####################################] <N>/<N> |
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.
If my nitpicking comments got included, need to modify the README.md a bit too.
@@ -26,7 +27,9 @@ def verify_deposit_data_json(filefolder: str) -> bool: | |||
""" | |||
with open(filefolder, 'r') as f: | |||
deposit_json = json.load(f) | |||
return all([validate_deposit(deposit) for deposit in deposit_json]) | |||
with click.progressbar(deposit_json, label='Verifying your deposits:\t', |
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.
with click.progressbar(deposit_json, label='Verifying your deposits:\t', | |
with click.progressbar(deposit_json, label='Verifying your deposit(s):\t', |
I decided to remove the "(s)" everywhere, because I don't expect there to be too many single deposits and even if there are, I don't think it is the end of the world so assume the plural. IMO "(s)" just looks ugly. |
Progress bars for key generation
When generating many keys, it can be frustrating not knowing how long it takes to generate your keys. This PR helps mitigate this frustration by offering progress bars: