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

Progress bars for key generation #144

Merged
merged 1 commit into from
Nov 3, 2020
Merged

Progress bars for key generation #144

merged 1 commit into from
Nov 3, 2020

Conversation

CarlBeek
Copy link
Collaborator

@CarlBeek CarlBeek commented Nov 3, 2020

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:

Creating your keys:               [####################################]  16/16          
Creating your keystores:          [####################################]  16/16          
Creating your depositdata:        [####################################]  16/16          
Verifying your keystores:         [####################################]  16/16          
Verifying your deposits:          [####################################]  16/16          

Success!

Copy link
Contributor

@hwwhww hwwhww left a 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',
Copy link
Contributor

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)"?

Suggested change
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',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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',

Comment on lines +114 to +118
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>
Copy link
Contributor

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',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
with click.progressbar(deposit_json, label='Verifying your deposits:\t',
with click.progressbar(deposit_json, label='Verifying your deposit(s):\t',

@CarlBeek
Copy link
Collaborator Author

CarlBeek commented Nov 3, 2020

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.

@CarlBeek CarlBeek merged commit 8fc1ce1 into dev Nov 3, 2020
@hwwhww hwwhww mentioned this pull request Nov 3, 2020
@CarlBeek CarlBeek deleted the progress_bars branch March 16, 2022 11:18
everhusk pushed a commit to earthwallet/earth-wallet-cli that referenced this pull request Aug 3, 2023
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