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

[WIP] Directory #186

Closed
wants to merge 2 commits into from
Closed

[WIP] Directory #186

wants to merge 2 commits into from

Conversation

garrensmith
Copy link
Contributor

This is the PR to get the directory layer working with the rust client. Its currently still very rough
and doesn't even compile yet.

If any of the maintainers want to contribute feel free to push code. I'm also on the rust discord (redcomet) if you want to chat about it.

@garrensmith
Copy link
Contributor Author

Just to add I've based the initial work off the golang and python implementation. I've also used the notes from the erlang implementation that we using for CouchDB.

@@ -6,7 +6,7 @@ fdb_rs_dir=$(pwd)

case $(uname) in
Darwin)
brew install mono
# brew install mono
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# brew install mono
command -v mono || brew install mono

@@ -22,7 +22,7 @@ esac
cd ${fdb_builddir:?}

## Get foundationdb source
git clone --depth 1 https://github.com/apple/foundationdb.git -b release-6.1
# git clone --depth 1 https://github.com/apple/foundationdb.git -b release-6.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# git clone --depth 1 https://github.com/apple/foundationdb.git -b release-6.1
[[ -d foundationdb ]] || git clone --depth 1 https://github.com/apple/foundationdb.git -b release-6.1

let key = self.subspace.subspace(&b"layer".to_vec());
self._layer = match trx.get(key.bytes(), false).await {
Ok(None) => Some(Vec::new()),
Err(_) => Some(Vec::new()),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think silencing an FdbError is ok.
layer should probably return a Result

subspace: Subspace,
path: Vec<String>,
target_path: Vec<String>,
_layer: Option<Vec<u8>>
Copy link
Contributor

Choose a reason for hiding this comment

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

Layer being used as a cache you might want to consider https://docs.rs/futures/0.3.4/futures/future/trait.FutureExt.html#method.shared.
RefCell might also be required in order to not requires &mut self on a lot of methods (but be careful about dynamic borrows lifespans in async context).

@PierreZ PierreZ mentioned this pull request Dec 31, 2020
12 tasks
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.

2 participants