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(cli): resume uploads across multiple safe runs #907

Merged
merged 14 commits into from Nov 7, 2023

Conversation

RolandSherwin
Copy link
Member

@RolandSherwin RolandSherwin commented Oct 26, 2023

Description

Summary generated by Reviewpad on 26 Oct 23 14:12 UTC

This pull request involves renaming the file "sn_cli/src/subcommands/files.rs" to "sn_cli/src/subcommands/files/mod.rs" and making changes to imports, module structure, function parameters, and method calls. Some code has been moved to a new module called "chunk_manager". The chunking functionality has been refactored and is now handled by the ChunkManager struct. The upload_files function has been modified to use the ChunkManager and no longer requires the wallet_dir_path parameter. The file_names_path has also been changed to use the root_dir. Some code related to logging has been commented out. Overall, the changes seem to improve the organization and modularity of the code.

The file wallet.rs has undergone several changes. Here is a summary of the changes made:

  • Added an import statement for ChunkManager from the files module.
  • Removed the import statement for chunk_path from the files module.
  • Replaced occurrences of wallet_dir_path with root_dir.
  • Replaced the file_api object initialization with Files::new(client.clone(), root_dir.to_path_buf()).
  • Replaced usage of chunk_path function with manager.chunk_path(&path)?.
  • Replaced getting all chunks from chunked_files.values() with manager.get_chunks().iter().
  • Removed the ChunkedFile struct definition and its associated code.

This file contains the implementation of a chunk manager, which is responsible for chunking files and managing the chunks. It includes several structs and methods for chunking files, storing chunk information, and retrieving chunks. The chunk manager takes a root directory and a files API as input and provides methods for chunking files, retrieving chunks, and marking finished chunks. The implementation uses external libraries such as color_eyre, rayon, sn_client, xor_name, walkdir for various functionalities.

Overall, this file provides the necessary functionality for chunking and managing files in the SAFE Network Software.

Please review these changes and consider their impact on the code. Let me know if you need more information.

@reviewpad reviewpad bot added the Large Large sized PR label Oct 26, 2023
@RolandSherwin RolandSherwin force-pushed the cli_resume branch 5 times, most recently from 1c0e56c to 69b2294 Compare October 31, 2023 12:12
@RolandSherwin RolandSherwin changed the title WIP: resume uploads across multiple safe runs feat(cli): resume uploads across multiple safe runs Oct 31, 2023
@RolandSherwin RolandSherwin marked this pull request as ready for review October 31, 2023 12:13
@reviewpad reviewpad bot requested a review from bochaco October 31, 2023 12:13
@RolandSherwin RolandSherwin force-pushed the cli_resume branch 6 times, most recently from 6abbb3d to 3e1dd7a Compare November 3, 2023 10:17
// use the chunk_artifacts_dir directly; This should not result in any
// undefined behaviour. The resume operation will be disabled if we don't
// use the `path_xor` dir.
// TODO: maybe error out if we get any fs errors.

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment

use std::fmt;

pub fn assert_lists<I, J, K>(a: I, b: J)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you elaborate or add docs here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added docs to that function.

Copy link
Member

Choose a reason for hiding this comment

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

maybe assert_list_eq would be a better name?

.iter()
.map(|(file_addr, chunked_file)| (*file_addr, chunked_file.file_name.clone()))
.collect::<Vec<_>>();
// Return early if we hav no files to pay/verify
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Return early if we hav no files to pay/verify
// Return early if we have no files to pay/verify

Copy link
Member

@grumbach grumbach left a comment

Choose a reason for hiding this comment

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

LGTM

@joshuef joshuef added this pull request to the merge queue Nov 7, 2023
Merged via the queue into maidsafe:main with commit 72b3f1e Nov 7, 2023
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Large Large sized PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants