)]}'
{"tvix/cli/src/tvix_store_io.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"e05c2731cfde5413c4aebfcfec3b909d2800c134","unresolved":true,"context_lines":[{"line_number":310,"context_line":"        .await"},{"line_number":311,"context_line":"        .expect(\"error during import_path\");"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"    // Render the NAR. This is blocking."},{"line_number":314,"context_line":"    let (nar_size, nar_sha256) \u003d"},{"line_number":315,"context_line":"        calculate_size_and_sha256(\u0026root_node, blob_service.clone(), directory_service.clone())"},{"line_number":316,"context_line":"            .await"}],"source_content_type":"text/x-rustsrc","patch_set":6,"id":"5491c9f1_1c5208a5","line":313,"updated":"2023-09-21 17:47:39.000000000","message":"This is not blocking anymore. While calculate_size_and_sha256 internally spawns a blocking task, import_path_with_pathinfo doesn\u0027t block anymore.","commit_id":"bbcbf496c80b6d97dca4177419fa9abd71194eec"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"8d27449920d95923f2e344b27ea469b9de3abd69","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        .await"},{"line_number":311,"context_line":"        .expect(\"error during import_path\");"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"    // Render the NAR. This is blocking."},{"line_number":314,"context_line":"    let (nar_size, nar_sha256) \u003d"},{"line_number":315,"context_line":"        calculate_size_and_sha256(\u0026root_node, blob_service.clone(), directory_service.clone())"},{"line_number":316,"context_line":"            .await"}],"source_content_type":"text/x-rustsrc","patch_set":6,"id":"23972f31_4daa0218","line":313,"in_reply_to":"5491c9f1_1c5208a5","updated":"2023-09-21 17:52:29.000000000","message":"Fixed","commit_id":"bbcbf496c80b6d97dca4177419fa9abd71194eec"}],"tvix/store/src/directoryservice/mod.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"97aafa469036c45c1f63ed8af154df01650c12c0","unresolved":true,"context_lines":[{"line_number":1,"context_line":"use std::pin::Pin;"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":"use crate::{proto, B3Digest, Error};"},{"line_number":4,"context_line":"use futures::Stream;"},{"line_number":5,"context_line":"use tonic::async_trait;"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"7f71821d_4008b582","line":2,"updated":"2023-09-21 10:14:17.000000000","message":"Can probably drop that newline and have it sort the imports.","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"e739f518504337f4abc7875403a6fb40219e5b71","unresolved":false,"context_lines":[{"line_number":1,"context_line":"use std::pin::Pin;"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":"use crate::{proto, B3Digest, Error};"},{"line_number":4,"context_line":"use futures::Stream;"},{"line_number":5,"context_line":"use tonic::async_trait;"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"d0496b15_dac97e68","line":2,"in_reply_to":"7f71821d_4008b582","updated":"2023-09-21 14:02:27.000000000","message":"Done","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"97aafa469036c45c1f63ed8af154df01650c12c0","unresolved":false,"context_lines":[{"line_number":23,"context_line":"#[async_trait]"},{"line_number":24,"context_line":"pub trait DirectoryService: Send + Sync {"},{"line_number":25,"context_line":"    /// Create a new instance by passing in a connection URL."},{"line_number":26,"context_line":"    /// TODO: check if we want to make this async, instead of lazily connecting"},{"line_number":27,"context_line":"    fn from_url(url: \u0026url::Url) -\u003e Result\u003cSelf, Error\u003e"},{"line_number":28,"context_line":"    where"},{"line_number":29,"context_line":"        Self: Sized;"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"b08b111a_fa27a518","line":26,"updated":"2023-09-21 10:14:17.000000000","message":"Agree - or removing that contrstuctor to elsewhere entirely, as discussed in DM (out of scope for this CL though)","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"}],"tvix/store/src/directoryservice/traverse.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"97aafa469036c45c1f63ed8af154df01650c12c0","unresolved":true,"context_lines":[{"line_number":12,"context_line":"/// clearly distinguish it from the BFS traversers."},{"line_number":13,"context_line":"#[instrument(skip(directory_service))]"},{"line_number":14,"context_line":"pub fn traverse_to("},{"line_number":15,"context_line":"    tokio_handle: tokio::runtime::Handle,"},{"line_number":16,"context_line":"    directory_service: Arc\u003cdyn DirectoryService\u003e,"},{"line_number":17,"context_line":"    node: crate::proto::node::Node,"},{"line_number":18,"context_line":"    path: \u0026std::path::Path,"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"21a3651b_906d1321","line":15,"updated":"2023-09-21 10:14:17.000000000","message":"Can we require this to just be in some runtime context where `tokio::runtime::Handle::current()` works (and add this requirement to the docstring), similar to how we do in `write_nar`? As you can see a bit further down in the testing code, this gets quite verbose.","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"e739f518504337f4abc7875403a6fb40219e5b71","unresolved":true,"context_lines":[{"line_number":12,"context_line":"/// clearly distinguish it from the BFS traversers."},{"line_number":13,"context_line":"#[instrument(skip(directory_service))]"},{"line_number":14,"context_line":"pub fn traverse_to("},{"line_number":15,"context_line":"    tokio_handle: tokio::runtime::Handle,"},{"line_number":16,"context_line":"    directory_service: Arc\u003cdyn DirectoryService\u003e,"},{"line_number":17,"context_line":"    node: crate::proto::node::Node,"},{"line_number":18,"context_line":"    path: \u0026std::path::Path,"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"f3b463e5_83067df2","line":15,"in_reply_to":"21a3651b_906d1321","updated":"2023-09-21 14:02:27.000000000","message":"The tricky thing is this function needs to run on a blocking thread (via `spawn_blocking`), but if its in a blocking context, you can\u0027t get the current tokio handle.\n\nIn `write_nar` I was able to hide this because it handles getting the tokio handle in the async context, then calls spawn_blocking on a private function and passes it the tokio handle.\n\nOne way to do this would be to split `traverse` to into two functions, one that does the tokio handle + spawn_blocking dance and one that actually does the work. I\u0027ll try that, let me know what you think.","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"2dc0e3357478525d7b74e3744679eeb318f7ad34","unresolved":false,"context_lines":[{"line_number":12,"context_line":"/// clearly distinguish it from the BFS traversers."},{"line_number":13,"context_line":"#[instrument(skip(directory_service))]"},{"line_number":14,"context_line":"pub fn traverse_to("},{"line_number":15,"context_line":"    tokio_handle: tokio::runtime::Handle,"},{"line_number":16,"context_line":"    directory_service: Arc\u003cdyn DirectoryService\u003e,"},{"line_number":17,"context_line":"    node: crate::proto::node::Node,"},{"line_number":18,"context_line":"    path: \u0026std::path::Path,"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"e9150fc4_16316482","line":15,"in_reply_to":"87409502_f0a23ae9","updated":"2023-09-21 15:29:30.000000000","message":"From the DMs, we decided to keep this async since it turns out we can write it fully in terms of async if we get rid of the recursion.","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"399eb8a56bd4c634d334bdcd71002ce25c8c096e","unresolved":true,"context_lines":[{"line_number":12,"context_line":"/// clearly distinguish it from the BFS traversers."},{"line_number":13,"context_line":"#[instrument(skip(directory_service))]"},{"line_number":14,"context_line":"pub fn traverse_to("},{"line_number":15,"context_line":"    tokio_handle: tokio::runtime::Handle,"},{"line_number":16,"context_line":"    directory_service: Arc\u003cdyn DirectoryService\u003e,"},{"line_number":17,"context_line":"    node: crate::proto::node::Node,"},{"line_number":18,"context_line":"    path: \u0026std::path::Path,"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"71fca9fb_2a117897","line":15,"in_reply_to":"f3b463e5_83067df2","updated":"2023-09-21 14:28:27.000000000","message":"Hmm, maybe this was better before. So here\u0027s the problem we\u0027re facing:\n- `tokio::runtime::Handle::current()` must be called in an async context (or it panics)\n- `tokio_handle.block_on()` must be called in a sync context (or it panics)\n\nThis means we need some coordination when crossing the boundary. For async-\u003esync we need to get the handle the in the async context and then pass the handle to the work inside `spawn_blocking`.\n\nFor sync-\u003easync boundary (like tvix_store_io), we already have a tokio handle that we created at initialization time, and we use that same handle to spawn async work.\n\nSo for `write_nar`, most of its callers are async. So we should provide the async function that does the `Handle::current()` -\u003e `spawn_blocking` -\u003e `tokio_handle.block_on` stuff internally. It looks and works like a regular async fn to the caller.\n\nFor `traverse_to`, the only caller is `tvix_store_io` which is sync and already has its own tokio_handle. In this case I think it makes sense to leave `traverse_to` as sync and expect the called to pass in their tokio handle. If we make it async (like the current patchset), `tvix_store_io` needs to do a `tokio::spawn` to call the function which seems like an extra step that\u0027s not needed.","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"bfc913ab16d9b93c36f9379a33db29939df231ca","unresolved":false,"context_lines":[{"line_number":12,"context_line":"/// clearly distinguish it from the BFS traversers."},{"line_number":13,"context_line":"#[instrument(skip(directory_service))]"},{"line_number":14,"context_line":"pub fn traverse_to("},{"line_number":15,"context_line":"    tokio_handle: tokio::runtime::Handle,"},{"line_number":16,"context_line":"    directory_service: Arc\u003cdyn DirectoryService\u003e,"},{"line_number":17,"context_line":"    node: crate::proto::node::Node,"},{"line_number":18,"context_line":"    path: \u0026std::path::Path,"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"87409502_f0a23ae9","line":15,"in_reply_to":"f3b463e5_83067df2","updated":"2023-09-21 14:29:41.000000000","message":"yeah, I think I like this more. thanks!","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"}],"tvix/store/src/fs/mod.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"97aafa469036c45c1f63ed8af154df01650c12c0","unresolved":true,"context_lines":[{"line_number":449,"context_line":""},{"line_number":450,"context_line":"                // Let the async task know we don\u0027t want any more entries."},{"line_number":451,"context_line":"                drop(rx);"},{"line_number":452,"context_line":"                self.tokio_handle.block_on(task).unwrap();"},{"line_number":453,"context_line":""},{"line_number":454,"context_line":"                return Ok(());"},{"line_number":455,"context_line":"            }"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"739b7d9d_0c216cf4","line":452,"updated":"2023-09-21 10:14:17.000000000","message":"Do we even need to block_on here? We already dropped rx, so the only thing this task is still gonna do is break out of the loop.\n\nI\u0027d probably drop that line, the `let task` above, and add a comment above the `.spawn` call explaining this will run in the background immediately.","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"e739f518504337f4abc7875403a6fb40219e5b71","unresolved":false,"context_lines":[{"line_number":449,"context_line":""},{"line_number":450,"context_line":"                // Let the async task know we don\u0027t want any more entries."},{"line_number":451,"context_line":"                drop(rx);"},{"line_number":452,"context_line":"                self.tokio_handle.block_on(task).unwrap();"},{"line_number":453,"context_line":""},{"line_number":454,"context_line":"                return Ok(());"},{"line_number":455,"context_line":"            }"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"40d523b8_dafaa093","line":452,"in_reply_to":"739b7d9d_0c216cf4","updated":"2023-09-21 14:02:27.000000000","message":"We probably don\u0027t. I was worried about maybe leaking tasks but its probably okay.","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"}],"tvix/store/src/fs/tests.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"97aafa469036c45c1f63ed8af154df01650c12c0","unresolved":true,"context_lines":[{"line_number":3,"context_line":"use std::os::unix::prelude::MetadataExt;"},{"line_number":4,"context_line":"use std::path::Path;"},{"line_number":5,"context_line":"use std::sync::Arc;"},{"line_number":6,"context_line":"use tokio::{fs, io};"},{"line_number":7,"context_line":"use tokio_stream::wrappers::ReadDirStream;"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"use tempfile::TempDir;"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"83442835_44db55f4","line":6,"updated":"2023-09-21 10:14:17.000000000","message":"This is more out of curiosity - did this fail when we tried to keep using sync io here?","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"297bbd9a0d5bed10c8ed330b0a9c56a25e74205e","unresolved":false,"context_lines":[{"line_number":3,"context_line":"use std::os::unix::prelude::MetadataExt;"},{"line_number":4,"context_line":"use std::path::Path;"},{"line_number":5,"context_line":"use std::sync::Arc;"},{"line_number":6,"context_line":"use tokio::{fs, io};"},{"line_number":7,"context_line":"use tokio_stream::wrappers::ReadDirStream;"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"use tempfile::TempDir;"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"4ef13db1_419916fb","line":6,"in_reply_to":"32c1dc5e_b8308b1e","updated":"2023-09-21 14:30:45.000000000","message":"hmmh, it could have been an instance of some tests not running on a multithreaded executor. had to do this in some places.\n\nusing async FS is fine however, was just curious.","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"e739f518504337f4abc7875403a6fb40219e5b71","unresolved":true,"context_lines":[{"line_number":3,"context_line":"use std::os::unix::prelude::MetadataExt;"},{"line_number":4,"context_line":"use std::path::Path;"},{"line_number":5,"context_line":"use std::sync::Arc;"},{"line_number":6,"context_line":"use tokio::{fs, io};"},{"line_number":7,"context_line":"use tokio_stream::wrappers::ReadDirStream;"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"use tempfile::TempDir;"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"32c1dc5e_b8308b1e","line":6,"in_reply_to":"83442835_44db55f4","updated":"2023-09-21 14:02:27.000000000","message":"Yeah, the tests would deadlock 😞","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"}],"tvix/store/src/nar/renderer.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"97aafa469036c45c1f63ed8af154df01650c12c0","unresolved":true,"context_lines":[{"line_number":33,"context_line":"///"},{"line_number":34,"context_line":"/// The writer is passed back in the return value. This is done because async Rust"},{"line_number":35,"context_line":"/// lacks scoped blocking tasks, so we need to transfer ownership of the writer"},{"line_number":36,"context_line":"/// internally."},{"line_number":37,"context_line":"pub async fn write_nar\u003cW: std::io::Write + Send + \u0027static\u003e("},{"line_number":38,"context_line":"    mut w: W,"},{"line_number":39,"context_line":"    proto_root_node: \u0026proto::node::Node,"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"498e02b3_75d66ead","line":36,"updated":"2023-09-21 10:14:17.000000000","message":"Add a line here explaining this needs to be called inside the context of a tokio runtime.","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"e739f518504337f4abc7875403a6fb40219e5b71","unresolved":false,"context_lines":[{"line_number":33,"context_line":"///"},{"line_number":34,"context_line":"/// The writer is passed back in the return value. This is done because async Rust"},{"line_number":35,"context_line":"/// lacks scoped blocking tasks, so we need to transfer ownership of the writer"},{"line_number":36,"context_line":"/// internally."},{"line_number":37,"context_line":"pub async fn write_nar\u003cW: std::io::Write + Send + \u0027static\u003e("},{"line_number":38,"context_line":"    mut w: W,"},{"line_number":39,"context_line":"    proto_root_node: \u0026proto::node::Node,"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"80d082a3_31e340b3","line":36,"in_reply_to":"498e02b3_75d66ead","updated":"2023-09-21 14:02:27.000000000","message":"Done","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"}],"tvix/store/src/pathinfoservice/grpc.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"97aafa469036c45c1f63ed8af154df01650c12c0","unresolved":false,"context_lines":[{"line_number":183,"context_line":"    }"},{"line_number":184,"context_line":"}"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"pub struct StreamIterator {"},{"line_number":187,"context_line":"    tokio_handle: tokio::runtime::Handle,"},{"line_number":188,"context_line":"    stream: Streaming\u003cproto::PathInfo\u003e,"},{"line_number":189,"context_line":"}"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"cf3c9990_e33c73c8","side":"PARENT","line":186,"updated":"2023-09-21 10:14:17.000000000","message":"Very happy to see this being gone :-)","commit_id":"7e737fde34260daa477794d63b0b3344b4a1d81b"}],"tvix/store/src/pathinfoservice/mod.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"97aafa469036c45c1f63ed8af154df01650c12c0","unresolved":true,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    /// Iterate over all PathInfo objects in the store."},{"line_number":50,"context_line":"    /// Implementations can decide to disallow listing."},{"line_number":51,"context_line":"    async fn list(\u0026self) -\u003e Pin\u003cBox\u003cdyn Stream\u003cItem \u003d Result\u003cproto::PathInfo, Error\u003e\u003e + Send\u003e\u003e;"},{"line_number":52,"context_line":"}"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"c0280ba7_2419a76c","line":51,"updated":"2023-09-21 10:14:17.000000000","message":"Why does this (now) return a Pin\u003cBox\u003c…\u003e\u003e? Can you add a comment?","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"e739f518504337f4abc7875403a6fb40219e5b71","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    /// Iterate over all PathInfo objects in the store."},{"line_number":50,"context_line":"    /// Implementations can decide to disallow listing."},{"line_number":51,"context_line":"    async fn list(\u0026self) -\u003e Pin\u003cBox\u003cdyn Stream\u003cItem \u003d Result\u003cproto::PathInfo, Error\u003e\u003e + Send\u003e\u003e;"},{"line_number":52,"context_line":"}"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"a6778e79_22fa812d","line":51,"in_reply_to":"c0280ba7_2419a76c","updated":"2023-09-21 14:02:27.000000000","message":"Added a comment, but we need the async form of an iterator, so a stream. This then has to be pinned so it can be easily polled later.\n\nThis is essentially what async_trait generates for async fns in traits, but for a `Stream` instead of a `Future`","commit_id":"11a45277dd9ba7bd13e5d4c6d015cb3fb4103ab7"}]}