Wp/ updated delete file to use pk as filename by rustchain64 · Pull Request #33 · bitmask-stack/carbonado-node

rustchain64 · 2023-04-19T01:32:16Z

Implements, getting the hash just after upload to then use a fetch get from the client to fetch that under RwLock

rustchain64 · 2023-04-19T01:34:10Z

At this stage, most of the features are working and tested,
However, even though get_slices was working last Friday, there is some kind of bug happening.
Also, mulitple different mime-types of many sizes needs to be uploaded and tested, and compared.

cryptoquick

Please clean up the "GOT HERE", "AM I HERE??", extra spaces, etc. Less git noise to review, and it makes the code less readable.

src/backend/fs.rs

Cargo.toml

cryptoquick · 2023-04-30T14:40:36Z

src/backend/fs.rs

+            let mut file = fs::File::create(&path)?;

+            // let mut file = OpenOptions::new()
+            //     .create_new(true)
+            //     .write(true)
+            //     .open(&path)?;


I'm not sure why you'd go with this approach. This overwrites the catalog. We should be checking if the catalog already exists in this code, but it seems this code also got commented out. Please put the other code back in, and let's use the old OpenOptions code here.

This solved an Internal Error 500 while using multi-part.
Good to point out it was over-writing the catalog.
This change has no value since we are not even using multi-part.

cryptoquick · 2023-04-30T14:42:14Z

src/backend/fs.rs

+    assert_eq!(std::io::ErrorKind::InvalidData, err.kind());
+
+    Ok(decoded)
+}


Why is this code here? Can't we just use extract_slice from carbonado core?

Removing this section, as I was using it for A/B testing on the results.

You have not yet removed it. Only request my review once everything has been addressed.

src/frontend/http.rs

src/backend/fs.rs

src/frontend/http.rs

tests/file.rs

…efactor file_stream to use explicit FileStream type alias

rustchain64 · 2023-05-03T03:46:25Z

This branch has been merged and lifted to accommodate
Streaming

* Stream `post_file` (#34) * Implement HTTP body streaming version of post_file. * Refactor streaming interface format. Fix tests. * Reduce number of files created in test by increasing chunk size to 1MB. * Fix check catalog_exists test. Cleanup unused libraries. * Log node public key in config. Correct Error Message in fs backend. Refactor file_stream to use explicit FileStream type alias (#35) * Update carbonado to 0.3.0-rc.7 and secp256k1 to 0.27.0. Fix misnamed variable in tests. * Add drive_redundancy option to config. * Update directories crate. Default volume directory is now the Carbonado folder in the home directory. Configurable drive redundancy. * Fixes in response to review items. --------- Co-authored-by: Wesley Parr <rustchain64@gmail.com> * Merge main. * Streaming improvements (#37) * Update unfold in write method to produce stream chunks of uniform segment size. * Parallelize write_file. Add config node_shared_secret method and /key endpoint. Improve tests. * Debugging... * Fixes. * Stream read file using axum response StreamBody. Parallelized decoding using par_then. * Cleanup. * Cleanup. --------- Co-authored-by: Wesley Parr <rustchain64@gmail.com>

cryptoquick

Still a number of things to address. Also, be sure there are no unused variables present.

cryptoquick · 2023-05-04T14:31:37Z

.DS_Store

Do not add this file

cryptoquick · 2023-05-04T14:31:51Z

file/body_test.png

Do not add this file, please remove

cryptoquick · 2023-05-04T14:32:11Z

src/backend/fs.rs

+    assert_eq!(std::io::ErrorKind::InvalidData, err.kind());
+
+    Ok(decoded)
+}


You have not yet removed it. Only request my review once everything has been addressed.

cryptoquick · 2023-05-04T14:32:44Z

src/backend/fs.rs

 }

-#[allow(unused_variables)]
+// TODO: These aren't worth breaking out into their own functions


Read this TODO item. Once addressed, remove it.

src/backend/fs.rs

cryptoquick · 2023-05-04T14:33:24Z

src/frontend/http.rs

    Ok((StatusCode::OK, hash.to_hex().to_string()))
 }

+/* TODO: This seems unnecessary, and could introduce concurrency and security issues


Please see this TODO. Once addressed, remove it.

rustchain64 · 2023-05-05T19:13:33Z

Test write_read() while file streaming doesn't finish before
read_write_delete_file()
This causes a count the show twice as many files since they haven't cleared before the second test.
To see accurate count numbers, you need to run the tests seperately.

cryptoquick

Good work so far on delete_file. Not so good work on read_slices.

Please update the description to annotate the issues this closes. The RwLock comment seems outdated.

cryptoquick · 2023-05-06T19:13:14Z

.DS_Store

This file still needs to be removed. You need to add that to your global gitignore (no need to add it to the project gitignore.)

added the (global) .gitignore for Mac

ran
sudo find / -name ".DS_Store" -depth -exec rm {} ;
ran cargo test, no .DS_Store present

cryptoquick · 2023-05-06T19:13:52Z

file/body_test.png

This file does not appear to be used, and besides, it's in the wrong place. Please remove this.

removed, not used for testing at this anymore

cryptoquick · 2023-05-06T19:32:27Z

src/backend/fs.rs

+    file_bytes: &[u8],
+    //slice_start: u64, //let slice_start = 65536;
+    //slice_len: u16,   // let slice_len = 8192;
+) -> Result<Vec<u8>> {


It makes no sense to provide the entire file to this method. This is supposed to be for retrieving just a slice of the file. It needs to return the range of the raw bytes on disk. It can make the disk IO calls by reading the catalog and then the appropriate segment.

I've updated the issue so hopefully the task is more clear:
#18

those values where for initial testing only.
actual slice data comes from the params.

cryptoquick · 2023-05-06T19:34:08Z

src/backend/fs.rs

+    use std::io::prelude::*;
+
+    // Start by encoding some input.
+    let (encoded_input, hash) = bao::encode::encode(file_bytes);
+    debug!(">>> encoded INPUT LENGTH:: {:?}", &encoded_input.len());
+    debug!(">>> fs hash hash:: {:?}", &hash);
+
+    // Slice the encoding. These parameters are multiples of the chunk size, which avoids
+    // unnecessary overhead.
+    let slice_start = 65536;
+    let slice_len = 8192;
+    let encoded_cursor = std::io::Cursor::new(&encoded_input);
+    let mut extractor = bao::encode::SliceExtractor::new(encoded_cursor, slice_start, slice_len);
+    let mut slice = Vec::new();
+    extractor.read_to_end(&mut slice)?;
+
+    // Decode the slice. The result should be the same as the part of the input that the slice
+    // represents. Note that we're using the same hash that encoding produced, which is
+    // independent of the slice parameters. That's the whole point; if we just wanted to re-encode
+    // a portion of the input and wind up with a different hash, we wouldn't need slicing.
+    let mut decoded = Vec::new();
+    let mut decoder = bao::decode::SliceDecoder::new(&*slice, &hash, slice_start, slice_len);
+    decoder.read_to_end(&mut decoded)?;
+
+    debug!(
+        "usize vs length: {:?}",
+        &encoded_input[slice_start as usize..][..slice_len as usize].len()
+    );
+
+    // Like regular decoding, slice decoding will fail if the hash doesn't match.
+    // let mut bad_slice = slice.clone();
+    // debug!("BAD SLICE");
+    // let last_index = bad_slice.len() - 1;
+    // bad_slice[last_index] ^= 1;
+    // let mut decoder = bao::decode::SliceDecoder::new(&*bad_slice, &hash, slice_start, slice_len);
+    // let err = decoder.read_to_end(&mut Vec::new()).unwrap_err();
+    // assert_eq!(std::io::ErrorKind::InvalidData, err.kind());
+
+    Ok(decoded)


None of this makes any sense. Do not submit this code again.

Reworking slices by reading catalog, not files as you stated.

cryptoquick · 2023-05-06T19:35:19Z

src/backend/fs.rs

-    for entry in fs::read_dir(path)? {
-        trace!("Delete CATALOG File at {:?}", entry);
-        fs::remove_file(entry?.path())?;
+pub async fn delete_file(hash: &String) -> Result<()> {


This should pass &str, not &String, this is not a pattern we use.

cryptoquick · 2023-05-06T19:54:12Z

src/frontend/http.rs

 pub async fn start() -> Result<()> {
    let app = Router::new()
-        .route("/remove/:pk/:blake3_hash", delete(remove_file))
+        .route("/remove/:blake3_hash", delete(remove_file))


You removed the pk... We'll need this to be passed for the changes I'm making in #39. I've trimmed that PR down so we can get that merged and you can use that to make sure file deletion is namespaced by pk.

Yes, your changes address that.

rustchain64 · 2023-05-08T12:24:51Z

While Testing Locally, test write_read ... FAILED
but passes remotely

test write_read ... FAILED

failures:

---- write_read stdout ----
Error: No such file or directory (os error 2)

Wesley Parr and others added 23 commits March 13, 2023 10:15

WP/skip files if exists on write

25eef4c

WP/check_catalog_exists [test]

61b2edc

WP/block write_read [test] temporarily

eddf0d5

WP/delete-file-blake3hash-http-route

07e5283

WP/delete-file-format-update

227ed37

WP/delete-file-fix-clippy-issues

7d85f6b

WP/18-slices-blake3_hash-slice_index-slice_range

e148dd9

WP/18-slice-blake3_hash-slice-prototype

8f30773

WP/18-slice-implement-http-get-slice

767156c

WP/18-slice-blake3_hash-slice-dev

f02bc40

WP/18-slice-blacke3-hash-slice-test-error-file-108

ab317e4

WP/18-slice-black3-hash-Encoded

908e39d

WP/18-slice-black3-hash-debug-stage

5f6c4c2

WP/18-slice-black3-HT-to-debug

ba92b5d

WP/18-slicke-blake3_hash-stash

684bb7c

WP/18-slice-blake3_hash-analyze-test

fc8634e

WP/18-slice-blake3-resolve-cargo-toml

29a31b4

WP/18-slice-blake3-clippy

e50a2d5

WP/18-slice-blake3-read_slices

6b77b22

WP/18-slice-blake3-test-update

657c120

WP/18-slice-blake3-unneeded-return

50dc264

WP/18-slice-blake3-http-get-slice

dedb85d

WP/upload-get-last-hash

13134f7

rustchain64 added 4 commits April 21, 2023 07:59

WP/upload-get-last-hash-debug

2f2e403

WP/upload-get-last-hash-cleanup

cbf7e9b

WP/upload-get-last-hash-test-to-pass

990420d

WP/upload-get-last-hash-file-read-test

eff5232

rustchain64 requested review from cryptoquick and josediegorobles April 25, 2023 17:06

rustchain64 added 2 commits April 29, 2023 11:38

WP/upload-get-last-point-cargo-toml

8e88ead

WP/upload-get-hash-clippy

9e3a446

cryptoquick requested changes Apr 30, 2023

View reviewed changes

cryptoquick and others added 6 commits April 30, 2023 11:25

Implement HTTP body streaming version of post_file.

8849101

Refactor streaming interface format. Fix tests.

ea0c994

Reduce number of files created in test by increasing chunk size to 1MB.

4629aa2

Fix check catalog_exists test. Cleanup unused libraries.

3e9949d

Log node public key in config. Correct Error Message in fs backend. R…

d1f9216

…efactor file_stream to use explicit FileStream type alias

WP/upload-last-hash-merger-with-stream-post-file

961f663

rustchain64 requested a review from cryptoquick May 2, 2023 15:34

cryptoquick and others added 2 commits May 4, 2023 08:25

Merge main.

7188500

cryptoquick requested changes May 4, 2023

View reviewed changes

rustchain64 added 4 commits May 4, 2023 10:11

temp stash

7db7d4d

update delete

4e21025

delete across volumes

642acb4

deleted files counter

8637f02

rustchain64 requested a review from cryptoquick May 5, 2023 18:21

comments

04d53bf

cryptoquick requested changes May 6, 2023

View reviewed changes

rustchain64 added 2 commits May 7, 2023 19:40

handle HT comments

04ef77f

updated delete file for file_name

dd5fa8c

rustchain64 changed the title ~~Wp/upload get last hash~~ Wp/ updated delete file to use pk as filename May 8, 2023

rustchain64 added 4 commits May 8, 2023 06:29

remove slice function

c675bd0

fix clippy error

18fc5df

read_slices backup

f651742

backup only

c4329ad

Conversation

rustchain64 commented Apr 19, 2023

Uh oh!

rustchain64 commented Apr 19, 2023

Uh oh!

cryptoquick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rustchain64 commented May 3, 2023

Uh oh!

cryptoquick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustchain64 commented May 5, 2023

Uh oh!

cryptoquick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustchain64 May 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustchain64 May 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustchain64 commented May 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

rustchain64 May 8, 2023 •

edited

Loading

rustchain64 May 8, 2023 •

edited

Loading

rustchain64 commented May 8, 2023 •

edited

Loading