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

refactor: Fallible store traits #2005

Merged
merged 11 commits into from
Feb 8, 2024
Merged

refactor: Fallible store traits #2005

merged 11 commits into from
Feb 8, 2024

Conversation

rklaehn
Copy link
Contributor

@rklaehn rklaehn commented Feb 7, 2024

Description

This makes the store traits fallible, which is a precondition for wiring up a store that uses IO for the blob index, such as the redb store.

There will be more changes to the traits, but this is necessary in any case and also a bit painful, so let's get it out of the way.

This also needs to extend the list-x responses with an error case, unfortunately.

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates if relevant.
  • Tests if relevant.

Use Self::POSTCARD_MAX_SIZE for fixed size
...to allow for stores that use io even for the lookup, such as a redb store.

Also use iroh_base::Hash instead of blake3::Hash, and make gc delete use batches.
let size = entry.outboard().await.ok()?.tree().size().0;
let path = "".to_owned();
Some(BlobListResponse { hash, size, path })
Gen::new(|co| async move {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this pattern is so common that we might want to do a helper for it. But maybe not just yet. It isn't that long.

iroh/src/sync_engine.rs Outdated Show resolved Hide resolved
# Conflicts:
#	iroh-bytes/src/get/db.rs
#	iroh/src/downloader/get.rs
@rklaehn rklaehn marked this pull request as ready for review February 7, 2024 14:06
@rklaehn rklaehn requested a review from Frando February 7, 2024 14:06
iroh/src/node.rs Outdated Show resolved Hide resolved
@rklaehn rklaehn changed the title Fallible store traits Refactor: Fallible store traits Feb 7, 2024
@rklaehn rklaehn changed the title Refactor: Fallible store traits refactor: Fallible store traits Feb 7, 2024
iroh/src/client.rs Outdated Show resolved Hide resolved
@rklaehn rklaehn added this pull request to the merge queue Feb 8, 2024
Merged via the queue into main with commit 1ad6510 Feb 8, 2024
18 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Feb 12, 2024
## Description

I did a a PR to merge the batch writer change. But it somehow got lost.
#2007

It was targeted to
[fallible-store-traits](#2005),
which got merged to main. I would have expected github to retarget the
PR to main, but apparently it did not do that. Maybe I made a mistake,
not sure exactly.

So here we go again.

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
fubuloubu pushed a commit to ApeWorX/iroh that referenced this pull request Feb 21, 2024
## Description

This makes the store traits fallible, which is a precondition for wiring
up a store that uses IO for the blob index, such as the redb store.

There will be more changes to the traits, but this is necessary in any
case and also a bit painful, so let's get it out of the way.

This also needs to extend the list-x responses with an error case,
unfortunately.

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [x] Self-review.
- [ ] Documentation updates if relevant.
- [x] Tests if relevant.
fubuloubu pushed a commit to ApeWorX/iroh that referenced this pull request Feb 21, 2024
## Description

I did a a PR to merge the batch writer change. But it somehow got lost.
n0-computer#2007

It was targeted to
[fallible-store-traits](n0-computer#2005),
which got merged to main. I would have expected github to retarget the
PR to main, but apparently it did not do that. Maybe I made a mistake,
not sure exactly.

So here we go again.

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
rklaehn added a commit to n0-computer/iroh-blobs that referenced this pull request Oct 22, 2024
## Description

I did a a PR to merge the batch writer change. But it somehow got lost.
n0-computer/iroh#2007

It was targeted to
[fallible-store-traits](n0-computer/iroh#2005),
which got merged to main. I would have expected github to retarget the
PR to main, but apparently it did not do that. Maybe I made a mistake,
not sure exactly.

So here we go again.

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants