Libify tlparse, first steps #3
Conversation
| let file = File::open(path)?; | ||
| let metadata = file.metadata()?; | ||
| let file_size = metadata.len(); | ||
|
|
||
| // TODO: abstract out this spinner to not be part of the library | ||
| // Instead, add a callback trait for CLIs to implement |
There was a problem hiding this comment.
@aorenste what do you think the best way to handle this is? The spinner gets updated on every loop of the log parse, but I don't want this in the library. It's also called on various errors with multi.suspend().
Maybe I could implement a trait like StatLogger and put the spinner in the callback in the CLI:
trait StatLogger {
fn init(&self) -> ();
fn log_stats(&mut self, bytes_read: u64, stat: &Stats) -> ();
// how to do **args in rust?
fn error(&self, e : Exception, **args) -> ();
}
pub fn parse_path_with_logger(path: &PathBuf, config: ParseConfig, logger: impl StatLogger) ...
// uses default no-op logger
pub fn parse_path(path: &PathBuf, config: ParseConfig) ...
And then call it later in the code when updating and exiting? Or is there a more rust-like way to do it?
There was a problem hiding this comment.
Yes - I think a trait is the right approach.
I would make sure to either make a trivial null implementation that callers can instantiate or have the call take Option<> so that callers don't have to build their own if they don't care.
Also you should take &mut impl StatLogger so it's borrowed instead of owned.
There was a problem hiding this comment.
Oh - I missed that you had two different APIs for it (one with and one without). I would just have one API that took Option<&mut impl StatLogger>.
There was a problem hiding this comment.
I'll do this in a separate PR: there's some complexities with the fact that you can't actually condition statically on impl StatLogger and also the spinner's side effects need to happen in the function initially. Going to land this change first
|
Talked with @aorenste offline, gonna rename a few things and test out cargo a bit more first |
|
|
||
| [[bin]] | ||
| name = "tlparse" | ||
| path = "src/cli.rs" |
|
Based on https://stackoverflow.com/questions/62861623/should-cargo-lock-be-committed-when-the-crate-is-both-a-rust-library-and-an-exec, I think we can keep the Cargo.lock as long as there's a lib.rs file for us to internally import. Crates that depend on the library will ignore the lock. |
cargo runorcargo build --bin tlparse) still results in an executable binary, but the library is now the main crate.TODO: need to test what happens with pip and cargo install