)]}'
{"tvix/castore/src/blobservice/object_store.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"3c58f3cad2ec2dfbf2d9e037d99ffed0bda22c4b","unresolved":true,"context_lines":[{"line_number":54,"context_line":"/// the literal contents of the chunk, but are zstd-compressed."},{"line_number":55,"context_line":"///"},{"line_number":56,"context_line":"/// ## Digest key sharding"},{"line_number":57,"context_line":"/// The blake3 digest encoded in lower hex, and sharded after the second"},{"line_number":58,"context_line":"/// character."},{"line_number":59,"context_line":"/// The blob for \"Hello World\" is stored at"},{"line_number":60,"context_line":"/// `${base_path}/blobs/b3/41/41f8394111eb713a22165c46c90ab8f0fd9399c92028fd6d288944b23ff5bf76`."}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"733bb8b0_c495a741","line":57,"updated":"2024-03-02 20:49:24.000000000","message":"considering we log out in base64 everywhere, base64 would probably also be ok?","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"29e33ebe8012f4af347bbe3f2bf82fa174d87c7b","unresolved":false,"context_lines":[{"line_number":54,"context_line":"/// the literal contents of the chunk, but are zstd-compressed."},{"line_number":55,"context_line":"///"},{"line_number":56,"context_line":"/// ## Digest key sharding"},{"line_number":57,"context_line":"/// The blake3 digest encoded in lower hex, and sharded after the second"},{"line_number":58,"context_line":"/// character."},{"line_number":59,"context_line":"/// The blob for \"Hello World\" is stored at"},{"line_number":60,"context_line":"/// `${base_path}/blobs/b3/41/41f8394111eb713a22165c46c90ab8f0fd9399c92028fd6d288944b23ff5bf76`."}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"10ded9e4_91310222","line":57,"in_reply_to":"733bb8b0_c495a741","updated":"2024-03-03 13:15:36.000000000","message":"base64 itself is a bit messy, as it contains \"/\".\n\n`object_store` is fine with it, it supports creating dirs as needed for its `file://` backend, but it\u0027s still annoying.\n\nWe could use urlsafe base64, though the whole point of it was being able to simply copy/paste hashes from logs, and that also wouldn\u0027t be given if we used that (and changing all logging to urlsafe base64 will mean these are not aligned with SRI.\n\nI\u0027ll just leave it hexlower for now, we can re-iterate on this later.","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"3c58f3cad2ec2dfbf2d9e037d99ffed0bda22c4b","unresolved":true,"context_lines":[{"line_number":278,"context_line":"        .then(|e| {"},{"line_number":279,"context_line":"            let base_path \u003d base_path.clone();"},{"line_number":280,"context_line":"            let object_store \u003d object_store.clone();"},{"line_number":281,"context_line":"            upload_chunk(e, base_path, object_store)"},{"line_number":282,"context_line":"        })"},{"line_number":283,"context_line":"        .collect::\u003cio::Result\u003cVec\u003cChunkMeta\u003e\u003e\u003e()"},{"line_number":284,"context_line":"        .await?;"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"5cf8bf23_62b6410b","line":281,"updated":"2024-03-02 20:49:24.000000000","message":"clones can be inlined now","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"29e33ebe8012f4af347bbe3f2bf82fa174d87c7b","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        .then(|e| {"},{"line_number":279,"context_line":"            let base_path \u003d base_path.clone();"},{"line_number":280,"context_line":"            let object_store \u003d object_store.clone();"},{"line_number":281,"context_line":"            upload_chunk(e, base_path, object_store)"},{"line_number":282,"context_line":"        })"},{"line_number":283,"context_line":"        .collect::\u003cio::Result\u003cVec\u003cChunkMeta\u003e\u003e\u003e()"},{"line_number":284,"context_line":"        .await?;"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"ce4d7a83_22870b07","line":281,"in_reply_to":"5cf8bf23_62b6410b","updated":"2024-03-03 13:15:36.000000000","message":"Done","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"3c58f3cad2ec2dfbf2d9e037d99ffed0bda22c4b","unresolved":false,"context_lines":[{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    let stat_blob_response \u003d StatBlobResponse {"},{"line_number":287,"context_line":"        chunks,"},{"line_number":288,"context_line":"        bao: \"\".into(), // still todo"},{"line_number":289,"context_line":"    };"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    // check for Blob, if it doesn\u0027t exist, persist."}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"908f4eca_917ee37a","line":288,"updated":"2024-03-02 20:49:24.000000000","message":"out of scope for this CL, that\u0027s something for verified streaming.","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"3c58f3cad2ec2dfbf2d9e037d99ffed0bda22c4b","unresolved":true,"context_lines":[{"line_number":321,"context_line":"    Ok(blob_digest)"},{"line_number":322,"context_line":"}"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"pin_project! {"},{"line_number":325,"context_line":"    struct B3Reader\u003cR\u003e"},{"line_number":326,"context_line":"    where"},{"line_number":327,"context_line":"        R: AsyncRead,"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"a8f2dd25_717a3342","line":324,"updated":"2024-03-02 20:49:24.000000000","message":"Missing docstring. Could potentially also be moved somewhere closer to where we define B3Digest (and return that)","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"29e33ebe8012f4af347bbe3f2bf82fa174d87c7b","unresolved":false,"context_lines":[{"line_number":321,"context_line":"    Ok(blob_digest)"},{"line_number":322,"context_line":"}"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"pin_project! {"},{"line_number":325,"context_line":"    struct B3Reader\u003cR\u003e"},{"line_number":326,"context_line":"    where"},{"line_number":327,"context_line":"        R: AsyncRead,"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"fb5567f9_e6fc6a9a","line":324,"in_reply_to":"a8f2dd25_717a3342","updated":"2024-03-03 13:15:36.000000000","message":"Done","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000082,"name":"Brian Olsen","display_name":"griff","email":"me@griff.name","username":"griff"},"change_message_id":"7defa42d2874a971a549bef1d3bc47826192d7b4","unresolved":true,"context_lines":[{"line_number":413,"context_line":""},{"line_number":414,"context_line":"        // poll the future, ensure it stays pending"},{"line_number":415,"context_line":"        let fut \u003d this.fut.as_pin_mut().expect(\"not future\");"},{"line_number":416,"context_line":"        let fut_p \u003d Box::pin(fut).poll_unpin(cx);"},{"line_number":417,"context_line":"        debug_assert!(fut_p.is_pending(), \"fut must keep pending\");"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"        w_p"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"76508f43_2fd661a8","line":416,"updated":"2024-03-03 01:43:16.000000000","message":"There is no need to box things here just call poll directly.\n```suggestion\n        let fut_p \u003d fut.poll(cx);\n```","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"29e33ebe8012f4af347bbe3f2bf82fa174d87c7b","unresolved":false,"context_lines":[{"line_number":413,"context_line":""},{"line_number":414,"context_line":"        // poll the future, ensure it stays pending"},{"line_number":415,"context_line":"        let fut \u003d this.fut.as_pin_mut().expect(\"not future\");"},{"line_number":416,"context_line":"        let fut_p \u003d Box::pin(fut).poll_unpin(cx);"},{"line_number":417,"context_line":"        debug_assert!(fut_p.is_pending(), \"fut must keep pending\");"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"        w_p"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"f328f812_2dd056d5","line":416,"in_reply_to":"76508f43_2fd661a8","updated":"2024-03-03 13:15:36.000000000","message":"Done","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000082,"name":"Brian Olsen","display_name":"griff","email":"me@griff.name","username":"griff"},"change_message_id":"6f58286075508bc01d145fe526fbe2d145d91887","unresolved":true,"context_lines":[{"line_number":414,"context_line":"        // poll the future, ensure it stays pending"},{"line_number":415,"context_line":"        let fut \u003d this.fut.as_pin_mut().expect(\"not future\");"},{"line_number":416,"context_line":"        let fut_p \u003d Box::pin(fut).poll_unpin(cx);"},{"line_number":417,"context_line":"        debug_assert!(fut_p.is_pending(), \"fut must keep pending\");"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"        w_p"},{"line_number":420,"context_line":"    }"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"a98edb4e_a505cd6f","line":417,"updated":"2024-03-03 10:51:45.000000000","message":"The future will return early if there is an error in the underlying upload. You can\u0027t just ignore that error either since the future will stop reading from the duplex and subsequent writes may fill the buffer and forever return pending.","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"29e33ebe8012f4af347bbe3f2bf82fa174d87c7b","unresolved":false,"context_lines":[{"line_number":414,"context_line":"        // poll the future, ensure it stays pending"},{"line_number":415,"context_line":"        let fut \u003d this.fut.as_pin_mut().expect(\"not future\");"},{"line_number":416,"context_line":"        let fut_p \u003d Box::pin(fut).poll_unpin(cx);"},{"line_number":417,"context_line":"        debug_assert!(fut_p.is_pending(), \"fut must keep pending\");"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"        w_p"},{"line_number":420,"context_line":"    }"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"18ba9463_4f4d4972","line":417,"in_reply_to":"a98edb4e_a505cd6f","updated":"2024-03-03 13:15:36.000000000","message":"I moved things around, poll the future first, and if it is ready return Poll::Ready(Err(…)) by myself.","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000082,"name":"Brian Olsen","display_name":"griff","email":"me@griff.name","username":"griff"},"change_message_id":"7defa42d2874a971a549bef1d3bc47826192d7b4","unresolved":true,"context_lines":[{"line_number":432,"context_line":""},{"line_number":433,"context_line":"        // poll the future, ensure it stays pending"},{"line_number":434,"context_line":"        let fut \u003d this.fut.as_pin_mut().expect(\"not future\");"},{"line_number":435,"context_line":"        let fut_p \u003d Box::pin(fut).poll_unpin(cx);"},{"line_number":436,"context_line":"        debug_assert!(fut_p.is_pending(), \"fut must keep pending\");"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        w_p"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"12cceec9_16f0a286","line":435,"updated":"2024-03-03 01:43:16.000000000","message":"Same as above no need to box things.","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"29e33ebe8012f4af347bbe3f2bf82fa174d87c7b","unresolved":false,"context_lines":[{"line_number":432,"context_line":""},{"line_number":433,"context_line":"        // poll the future, ensure it stays pending"},{"line_number":434,"context_line":"        let fut \u003d this.fut.as_pin_mut().expect(\"not future\");"},{"line_number":435,"context_line":"        let fut_p \u003d Box::pin(fut).poll_unpin(cx);"},{"line_number":436,"context_line":"        debug_assert!(fut_p.is_pending(), \"fut must keep pending\");"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        w_p"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"27553a7e_68b82b7c","line":435,"in_reply_to":"12cceec9_16f0a286","updated":"2024-03-03 13:15:36.000000000","message":"Done","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000082,"name":"Brian Olsen","display_name":"griff","email":"me@griff.name","username":"griff"},"change_message_id":"6f58286075508bc01d145fe526fbe2d145d91887","unresolved":true,"context_lines":[{"line_number":433,"context_line":"        // poll the future, ensure it stays pending"},{"line_number":434,"context_line":"        let fut \u003d this.fut.as_pin_mut().expect(\"not future\");"},{"line_number":435,"context_line":"        let fut_p \u003d Box::pin(fut).poll_unpin(cx);"},{"line_number":436,"context_line":"        debug_assert!(fut_p.is_pending(), \"fut must keep pending\");"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        w_p"},{"line_number":439,"context_line":"    }"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"bcb77eca_a6e91b6b","line":436,"updated":"2024-03-03 10:51:45.000000000","message":"Same as above future might return early with an error.","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"29e33ebe8012f4af347bbe3f2bf82fa174d87c7b","unresolved":false,"context_lines":[{"line_number":433,"context_line":"        // poll the future, ensure it stays pending"},{"line_number":434,"context_line":"        let fut \u003d this.fut.as_pin_mut().expect(\"not future\");"},{"line_number":435,"context_line":"        let fut_p \u003d Box::pin(fut).poll_unpin(cx);"},{"line_number":436,"context_line":"        debug_assert!(fut_p.is_pending(), \"fut must keep pending\");"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        w_p"},{"line_number":439,"context_line":"    }"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"a240738e_db9d7e40","line":436,"in_reply_to":"bcb77eca_a6e91b6b","updated":"2024-03-03 13:15:36.000000000","message":"Acknowledged","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000082,"name":"Brian Olsen","display_name":"griff","email":"me@griff.name","username":"griff"},"change_message_id":"7defa42d2874a971a549bef1d3bc47826192d7b4","unresolved":true,"context_lines":[{"line_number":458,"context_line":"        match self.writer.take() {"},{"line_number":459,"context_line":"            Some(writer) \u003d\u003e {"},{"line_number":460,"context_line":"                // drop the writer explicitly, so the other side will read EOF."},{"line_number":461,"context_line":"                drop(writer);"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"                // take out the future."},{"line_number":464,"context_line":"                let fut \u003d self.fut.take().expect(\"fut must be some\");"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"e3519245_b7fa55e6","line":461,"updated":"2024-03-03 01:43:16.000000000","message":"It is not enough to just drop the writer. You must always call poll_shutdown on an AsyncWrite otherwise the code may sometimes drop data. The tricky thing is it might work sometimes and with some writers all the time but to guarantee that all data is correctly processed, written and flushed you must call poll_shutdown. \n```suggestion\n                writer.shutdown().await?;\n```","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"29e33ebe8012f4af347bbe3f2bf82fa174d87c7b","unresolved":false,"context_lines":[{"line_number":458,"context_line":"        match self.writer.take() {"},{"line_number":459,"context_line":"            Some(writer) \u003d\u003e {"},{"line_number":460,"context_line":"                // drop the writer explicitly, so the other side will read EOF."},{"line_number":461,"context_line":"                drop(writer);"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"                // take out the future."},{"line_number":464,"context_line":"                let fut \u003d self.fut.take().expect(\"fut must be some\");"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"7e387c3b_5c968496","line":461,"in_reply_to":"e3519245_b7fa55e6","updated":"2024-03-03 13:15:36.000000000","message":"Ok, thanks. Done!","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"3c58f3cad2ec2dfbf2d9e037d99ffed0bda22c4b","unresolved":true,"context_lines":[{"line_number":467,"context_line":""},{"line_number":468,"context_line":"                // store it in fut_output, so we won\u0027t pull the future"},{"line_number":469,"context_line":"                // after completion."},{"line_number":470,"context_line":"                // TODO: cloneable error, so we can store it"},{"line_number":471,"context_line":"                // self.fut_output \u003d Some(fut_output);"},{"line_number":472,"context_line":"                fut.await"},{"line_number":473,"context_line":"            }"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"6f889a84_5eb9aad6","line":470,"updated":"2024-03-02 20:49:24.000000000","message":"it\u0027s probably ok to not return the same error multiple times, we can just return a more generic error when calling close for the second time.","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000082,"name":"Brian Olsen","display_name":"griff","email":"me@griff.name","username":"griff"},"change_message_id":"7defa42d2874a971a549bef1d3bc47826192d7b4","unresolved":true,"context_lines":[{"line_number":467,"context_line":""},{"line_number":468,"context_line":"                // store it in fut_output, so we won\u0027t pull the future"},{"line_number":469,"context_line":"                // after completion."},{"line_number":470,"context_line":"                // TODO: cloneable error, so we can store it"},{"line_number":471,"context_line":"                // self.fut_output \u003d Some(fut_output);"},{"line_number":472,"context_line":"                fut.await"},{"line_number":473,"context_line":"            }"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"e530e1d6_4ebf0dca","line":470,"in_reply_to":"6f889a84_5eb9aad6","updated":"2024-03-03 01:43:16.000000000","message":"When I have needed to clone an io::Error I have usually just made a new one with the same ErrorKind and message.","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"29e33ebe8012f4af347bbe3f2bf82fa174d87c7b","unresolved":false,"context_lines":[{"line_number":467,"context_line":""},{"line_number":468,"context_line":"                // store it in fut_output, so we won\u0027t pull the future"},{"line_number":469,"context_line":"                // after completion."},{"line_number":470,"context_line":"                // TODO: cloneable error, so we can store it"},{"line_number":471,"context_line":"                // self.fut_output \u003d Some(fut_output);"},{"line_number":472,"context_line":"                fut.await"},{"line_number":473,"context_line":"            }"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"ca7e33f9_555b876f","line":470,"in_reply_to":"e530e1d6_4ebf0dca","updated":"2024-03-03 13:15:36.000000000","message":"Done","commit_id":"279c284f682bca4b3071bdde7688284e5f2d2dd4"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"2bf3f48bedb7efe83ac65d2dd18b9fed776211f7","unresolved":true,"context_lines":[{"line_number":402,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":403,"context_line":"        // There\u0027s nothing to do on shutdown. We might have written some chunks"},{"line_number":404,"context_line":"        // that are nowhere else referenced, but cleaning them up here would be racy."},{"line_number":405,"context_line":"        std::task::Poll::Ready(Ok(()))"},{"line_number":406,"context_line":"    }"},{"line_number":407,"context_line":"}"},{"line_number":408,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":5,"id":"86220acc_7d325912","line":405,"updated":"2024-03-03 14:58:07.000000000","message":"@me@griff.name just to make sure, us not calling shutdown on our `writer` here causes `upload_blob` to just get stopped, as in, it never sees a \"proper EOF\", so it doesn\u0027t persist the StatBlobResponse (and might just leave unreferenced chunks around)?","commit_id":"3e9967da244fcb34124a3a1163fde9ab9335a680"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"759363a0eaaadcc65deafe409055a7b6dda744b2","unresolved":false,"context_lines":[{"line_number":402,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":403,"context_line":"        // There\u0027s nothing to do on shutdown. We might have written some chunks"},{"line_number":404,"context_line":"        // that are nowhere else referenced, but cleaning them up here would be racy."},{"line_number":405,"context_line":"        std::task::Poll::Ready(Ok(()))"},{"line_number":406,"context_line":"    }"},{"line_number":407,"context_line":"}"},{"line_number":408,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":5,"id":"d46cdadf_d0bf9831","line":405,"in_reply_to":"5117a5ec_0ead2216","updated":"2024-03-11 21:17:53.000000000","message":"Stray chunks are totally fine, it\u0027d be worse to have to figure out which ones we can remove in case an upload is cancelled (and do that race-free)","commit_id":"3e9967da244fcb34124a3a1163fde9ab9335a680"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"03302857d6dac8b7d409866a8aceb5696042ebf9","unresolved":true,"context_lines":[{"line_number":402,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":403,"context_line":"        // There\u0027s nothing to do on shutdown. We might have written some chunks"},{"line_number":404,"context_line":"        // that are nowhere else referenced, but cleaning them up here would be racy."},{"line_number":405,"context_line":"        std::task::Poll::Ready(Ok(()))"},{"line_number":406,"context_line":"    }"},{"line_number":407,"context_line":"}"},{"line_number":408,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":5,"id":"7f551999_51811f6c","line":405,"in_reply_to":"63748591_5ecc2ca7","updated":"2024-03-03 16:59:32.000000000","message":"That\u0027s not what I meant with my question - Assume a client uses `BlobService::open_write()`, then halfway stops writing (because the process dies, or couldn\u0027t get all the data it wanted to write, …), doesn\u0027t matter - it does not call `close()`.\n\nIn that case we don\u0027t want the blob to be finalized, aka nothing should be written in `blobs/…` (EOF code handling of `upload_blob`), only some stray `chunks/…` might be stay sitting around.\n\nDoes the code behave like this currently, or does `upload_blob` see a EOF in these cases too? If that\u0027s the case, we might need to signal differently that the upload should be aborted…","commit_id":"3e9967da244fcb34124a3a1163fde9ab9335a680"},{"author":{"_account_id":1000082,"name":"Brian Olsen","display_name":"griff","email":"me@griff.name","username":"griff"},"change_message_id":"acf6545dabc336394d6529381d2c3d41b1231ef6","unresolved":false,"context_lines":[{"line_number":402,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":403,"context_line":"        // There\u0027s nothing to do on shutdown. We might have written some chunks"},{"line_number":404,"context_line":"        // that are nowhere else referenced, but cleaning them up here would be racy."},{"line_number":405,"context_line":"        std::task::Poll::Ready(Ok(()))"},{"line_number":406,"context_line":"    }"},{"line_number":407,"context_line":"}"},{"line_number":408,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":5,"id":"5117a5ec_0ead2216","line":405,"in_reply_to":"7f551999_51811f6c","updated":"2024-03-11 18:28:47.000000000","message":"When aborting the common thing is to simply stop polling the future and maybe dropping it. Most futures are built to be aborted this way and if the user of BlobService detects that the other process stops writing with a timeout and then gets dropped you should be fine.\n\nDepending on how atomic object store is you should get some stray chunks if it is atomic and some unfinished files laying around if it isn\u0027t atomic.","commit_id":"3e9967da244fcb34124a3a1163fde9ab9335a680"},{"author":{"_account_id":1000082,"name":"Brian Olsen","display_name":"griff","email":"me@griff.name","username":"griff"},"change_message_id":"3865d73b8ca59713bc415cdf9481b473e2e95b59","unresolved":true,"context_lines":[{"line_number":402,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":403,"context_line":"        // There\u0027s nothing to do on shutdown. We might have written some chunks"},{"line_number":404,"context_line":"        // that are nowhere else referenced, but cleaning them up here would be racy."},{"line_number":405,"context_line":"        std::task::Poll::Ready(Ok(()))"},{"line_number":406,"context_line":"    }"},{"line_number":407,"context_line":"}"},{"line_number":408,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":5,"id":"63748591_5ecc2ca7","line":405,"in_reply_to":"86220acc_7d325912","updated":"2024-03-03 16:49:02.000000000","message":"As long as close is called it is fine. The documentation should probably mention that calling close on a BlobWriter is mandatory and that not calling it can lead to inconsistencies.","commit_id":"3e9967da244fcb34124a3a1163fde9ab9335a680"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"d04d603a2acc0e384b461d52165d8c61d4d24f17","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        if digest.as_slice() \u003d\u003d blake3::hash(b\"\").as_bytes() {"},{"line_number":135,"context_line":"            return Ok(Some(Box::new(Cursor::new(b\"\")) as Box\u003cdyn BlobReader\u003e));"},{"line_number":136,"context_line":"        }"},{"line_number":137,"context_line":"        Ok("},{"line_number":138,"context_line":"            match self"},{"line_number":139,"context_line":"                .object_store"},{"line_number":140,"context_line":"                .get(\u0026derive_chunk_path(\u0026self.base_path, digest))"}],"source_content_type":"text/x-rustsrc","patch_set":9,"id":"63a58966_dbad9aa8","line":137,"updated":"2024-03-11 22:07:27.000000000","message":"nit: Maybe push the `Ok()` down to each match arm to reduce some nesting and avoid needing to do `Err(e)?`","commit_id":"d21d882e9fd3f362af2095b2ce453134b167178b"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"cb6413d7b38250486f3447385a737da34951ab8f","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        if digest.as_slice() \u003d\u003d blake3::hash(b\"\").as_bytes() {"},{"line_number":135,"context_line":"            return Ok(Some(Box::new(Cursor::new(b\"\")) as Box\u003cdyn BlobReader\u003e));"},{"line_number":136,"context_line":"        }"},{"line_number":137,"context_line":"        Ok("},{"line_number":138,"context_line":"            match self"},{"line_number":139,"context_line":"                .object_store"},{"line_number":140,"context_line":"                .get(\u0026derive_chunk_path(\u0026self.base_path, digest))"}],"source_content_type":"text/x-rustsrc","patch_set":9,"id":"817f92f5_096f307e","line":137,"in_reply_to":"63a58966_dbad9aa8","updated":"2024-03-11 22:25:13.000000000","message":"Done.\n\nErr(e)? also took care of the e.into() we now have, but the nesting is an argument.","commit_id":"d21d882e9fd3f362af2095b2ce453134b167178b"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"d04d603a2acc0e384b461d52165d8c61d4d24f17","unresolved":true,"context_lines":[{"line_number":213,"context_line":"    async fn chunks(\u0026self, digest: \u0026B3Digest) -\u003e io::Result\u003cOption\u003cVec\u003cChunkMeta\u003e\u003e\u003e {"},{"line_number":214,"context_line":"        let p \u003d derive_blob_path(\u0026self.base_path, digest);"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        Ok(match self.object_store.get(\u0026p).await {"},{"line_number":217,"context_line":"            Ok(get_result) \u003d\u003e {"},{"line_number":218,"context_line":"                // fetch the data at the blob path"},{"line_number":219,"context_line":"                let blob_data \u003d get_result.bytes().await?;"}],"source_content_type":"text/x-rustsrc","patch_set":9,"id":"95a805e5_ea2fd7cc","line":216,"updated":"2024-03-11 22:07:27.000000000","message":"ditto","commit_id":"d21d882e9fd3f362af2095b2ce453134b167178b"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"cb6413d7b38250486f3447385a737da34951ab8f","unresolved":false,"context_lines":[{"line_number":213,"context_line":"    async fn chunks(\u0026self, digest: \u0026B3Digest) -\u003e io::Result\u003cOption\u003cVec\u003cChunkMeta\u003e\u003e\u003e {"},{"line_number":214,"context_line":"        let p \u003d derive_blob_path(\u0026self.base_path, digest);"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        Ok(match self.object_store.get(\u0026p).await {"},{"line_number":217,"context_line":"            Ok(get_result) \u003d\u003e {"},{"line_number":218,"context_line":"                // fetch the data at the blob path"},{"line_number":219,"context_line":"                let blob_data \u003d get_result.bytes().await?;"}],"source_content_type":"text/x-rustsrc","patch_set":9,"id":"903f559a_cc222c9d","line":216,"in_reply_to":"95a805e5_ea2fd7cc","updated":"2024-03-11 22:25:13.000000000","message":"Done","commit_id":"d21d882e9fd3f362af2095b2ce453134b167178b"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"d04d603a2acc0e384b461d52165d8c61d4d24f17","unresolved":true,"context_lines":[{"line_number":388,"context_line":"        }"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"        // write to the underlying writer"},{"line_number":391,"context_line":"        let w_p \u003d this"},{"line_number":392,"context_line":"            .writer"},{"line_number":393,"context_line":"            .as_pin_mut()"},{"line_number":394,"context_line":"            .expect(\"writer must be some\")"}],"source_content_type":"text/x-rustsrc","patch_set":9,"id":"f10ab0c3_112b2b5c","line":391,"updated":"2024-03-11 22:07:27.000000000","message":"why do we need the `w_p` binding here? doesn\u0027t look like we do anything with it","commit_id":"d21d882e9fd3f362af2095b2ce453134b167178b"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"cb6413d7b38250486f3447385a737da34951ab8f","unresolved":false,"context_lines":[{"line_number":388,"context_line":"        }"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"        // write to the underlying writer"},{"line_number":391,"context_line":"        let w_p \u003d this"},{"line_number":392,"context_line":"            .writer"},{"line_number":393,"context_line":"            .as_pin_mut()"},{"line_number":394,"context_line":"            .expect(\"writer must be some\")"}],"source_content_type":"text/x-rustsrc","patch_set":9,"id":"d9c4ab00_048da2da","line":391,"in_reply_to":"f10ab0c3_112b2b5c","updated":"2024-03-11 22:25:13.000000000","message":"Nice catch (here and in flush too). I think I did the future polling afterwards initially, and that was why I had to do the let binding first.","commit_id":"d21d882e9fd3f362af2095b2ce453134b167178b"}]}