feat: offline adding #415
Merged
feat: offline adding #415
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rklaehn
previously approved these changes
Oct 25, 2022
| // we require p2p for adding right now because we don't have a mechanism for | ||
| // hydrating only the root CID to the p2p node for providing if a CID were | ||
| // ingested offline. Offline adding should happen, but this is the current | ||
| // path of least confusion | ||
| require_services(api, HashSet::from(["store", "p2p"])).await?; | ||
| let svc_status = require_services(api, HashSet::from(["store"])).await?; | ||
| match (provide, svc_status.p2p.status()) { |
There was a problem hiding this comment.
Not for this PR, but we have to make sure that we refuse to speak to services with different build version
iroh/src/run.rs
Outdated
| (true, ServiceStatus::Unknown) | ||
| | (true, ServiceStatus::NotServing) | ||
| | (true, ServiceStatus::ServiceUnknown) => { | ||
| return Err(anyhow!("Add provides content to the IPFS network by default, but the p2p service is not running.\n{}", |
There was a problem hiding this comment.
There is anyhow::bail! which does this and maybe looks a bit nicer.
| /// services for additional scrutiny | ||
| pub async fn require_services( | ||
| api: &impl Api, | ||
| services: HashSet<&str>, |
There was a problem hiding this comment.
I would prefer BTreeSet in such cases because it is more deterministic. But in this case it does not really matter.
There was a problem hiding this comment.
yeah totally. I'll come through & refactor all uses of HashSet to BTreeSet in a follow up
| path: &Path, | ||
| no_wrap: bool, | ||
| recursive: bool, | ||
| provide: bool, |
There was a problem hiding this comment.
the next person touching this needs to create a config struct
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have no idea what this does for our provide story if a user adds content offline then tries to take it online, but this makes our perf measurable.
I still think we should provide the root to the DHT by default. It's usually what users are trying to do, and while it's slower, it's the de-facto use case