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

feat: use progress bars on files upload #801

Merged
merged 3 commits into from Oct 5, 2023

Conversation

jacderida
Copy link
Contributor

@jacderida jacderida commented Oct 5, 2023

  • 7d95d86 refactor: pay_for_chunks returns cost and new balance

    Rather than the client printing out information relating to the cost of the chunks and the resulting
    new balance, the information is returned to the caller, who can then print it out.

    The error handling for storing the wallet is also changed to propagate the error rather than
    suppressing it by printing output.

  • ce3c585 refactor: use one files api and clarify variable names

    The file_api was being created multiple times when it didn't seem to need to be. It is easier to
    pass around the file_api that you create once, rather than the client and the wallet directory
    path.

    I also renamed root_dir to wallet_dir_path because there didn't seem to be any need for it to
    not be more specific as to what it actually refers to.

  • 6665ff2 feat: use progress bars on files upload

    Change the user interface for the files upload command to use progress bars to indicate the status
    of the following operations:

    • Splitting the input files into chunks
    • Uploading the split chunks
    • Verifying the chunks that were uploaded
    • Re-uploading any chunks that failed verification

    It uses a simple approach, which is to create a new ProgressBar for each of the operations, rather
    than attempt to use a MultiBar or re-use the same progress bar. In this simple approach, the
    finish_and_clear function removes the bar from stderr, then a new bar is created for the next
    operation.

    The println! output from the client was removed in favour of returning that information back to
    the caller.

    There is much less text in the output of the command now, but it hopefully communicates everything
    that's important.

Description

Summary generated by Reviewpad on 05 Oct 23 00:05 UTC

This pull request includes the following changes:

  1. The NanoTokens struct has been imported from the sn_transfers module.
  2. The pay_for_chunks method now returns the cost and new balance of the local wallet.
  3. The documentation for the pay_for_chunks method has been improved.

The "Justfile" script has been modified to add a new task called "run-local-network". This task kills the processes "safenode" and "faucet", removes the directory "~/.local/share/safe", and then builds the testnet with local discovery using "cargo" with an interval of 1000.

In the "Cargo.toml" file, the following changes have been made:

  • Added dependency indicatif with version 0.17.5 and features ["tokio"].
  • Updated dependency libp2p to version 0.52 and added features ["identify", "kad"].
  • Updated dependency reqwest to version 0.11.18 with default features disabled and added feature ["rustls"].

In the wallet.rs file, there are several changes:

  1. The root_dir parameter has been replaced with wallet_dir_path.
  2. The send, receive, and get_faucet functions now include the wallet_dir_path parameter.
  3. The WalletCmds::Pay variant has been modified to include a path parameter and batch_size field.
  4. The chunk_path function now takes a file_api parameter instead of client and root_dir.
  5. The creation of file_api has been moved outside the WalletCmds::Pay variant.

The "indicatif" dependency has been added to the Cargo.lock file.

In the local_store.rs file, the println!("Transfers applied locally"); statement has been removed from the update_local_wallet method of the LocalWallet struct.

The wallet.rs file has the following changes:

  • The import statement for Instant has been removed.
  • The variable num_of_payments has been removed.
  • The variable now and its usage in time tracking have been removed.
  • The print statements for transfer completion and total payment have been removed.

Please let me know if you need more information on any specific lines of code.

Rather than the client printing out information relating to the cost of the chunks and the resulting
new balance, the information is returned to the caller, who can then print it out.

The error handling for storing the wallet is also changed to propagate the error rather than
suppressing it by printing output.
The `file_api` was being created multiple times when it didn't seem to need to be. It is easier to
pass around the `file_api` that you create once, rather than the client and the wallet directory
path.

I also renamed `root_dir` to `wallet_dir_path` because there didn't seem to be any need for it to
not be more specific as to what it actually refers to.
Change the user interface for the `files upload` command to use progress bars to indicate the status
of the following operations:

* Splitting the input files into chunks
* Uploading the split chunks
* Verifying the chunks that were uploaded
* Re-uploading any chunks that failed verification

It uses a simple approach, which is to create a new `ProgressBar` for each of the operations, rather
than attempt to use a `MultiBar` or re-use the same progress bar. In this simple approach, the
`finish_and_clear` function removes the bar from stderr, then a new bar is created for the next
operation.

The `println!` output from the client was removed in favour of returning that information back to
the caller.

There is much less text in the output of the command now, but it hopefully communicates everything
that's important.
@reviewpad reviewpad bot requested a review from bochaco October 5, 2023 00:04
@reviewpad reviewpad bot added Medium Medium sized PR waiting-for-review labels Oct 5, 2023
@joshuef joshuef added this pull request to the merge queue Oct 5, 2023
Merged via the queue into maidsafe:main with commit 4e745d4 Oct 5, 2023
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Medium Medium sized PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants