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

Prefactor: Extract store and sharded_lmdb into their own crates #7904

Conversation

illicitonion
Copy link
Contributor

@illicitonion illicitonion commented Jun 19, 2019

I'm about to introduce a second wrapper around ShardedLmdb separate from Store, and this will make it easier to introduce this.

This is just a set of extractions of types; there are no functional changes here.

@illicitonion illicitonion force-pushed the dwagnerhall/local-process-execution-cache branch from 3a18f8b to 2cf146d Compare June 19, 2019 23:32
@illicitonion illicitonion force-pushed the dwagnerhall/local-process-execution-cache branch from 2cf146d to bfc7751 Compare June 20, 2019 08:14
@blorente blorente self-requested a review June 20, 2019 09:41
Copy link
Contributor

@blorente blorente left a comment

Choose a reason for hiding this comment

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

LGTM, small questions rather than suggestions :)

Err(blocking_err) => Err(format!("Unable to run blocking task to load_bytes in local ByteStore on tokio runtime: {}", blocking_err)),
}
}).to_boxed()
let (env, db, _) = dbs.clone()?.get(&digest.0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this formatting change intentional? I'm assuming it is, but it looks suspiciously like IntelliJ messing things up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is whatever rustfmt did for me. That said, unindenting it was fine by rustfmt too, so... Ugh, I miss formatters which have one true style :)

use std::sync::Arc;
use std::time;
use tempfile::TempDir;

use super::MAX_LOCAL_STORE_SIZE_BYTES;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little bit confused as to why this lives here, is this constant fs-dependent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved some things around, added some comments

@illicitonion illicitonion merged commit 6890cc0 into pantsbuild:master Jun 20, 2019
@illicitonion illicitonion deleted the dwagnerhall/local-process-execution-cache branch June 20, 2019 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants