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: client Entry with methods to read content #1854

Merged
merged 3 commits into from
Dec 12, 2023

Conversation

Frando
Copy link
Member

@Frando Frando commented Nov 29, 2023

Description

This adds a iroh::client::Entry struct which wraps the iroh_sync::Entry and adds methods to read the content of the entry. Reading the entry needs a reference to either a Doc<C> or the Iroh<C> client (both work equally).

@flub mentioned that this is missing from the expected API. I started this at the hack session but didn't get very far but now it's in a good shape IMO.

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates if relevant.

@Frando Frando force-pushed the refactor-client-entry branch from 6868b48 to eafc6a7 Compare November 29, 2023 17:49
@dignifiedquire
Copy link
Contributor

why not store the client on the Entry?

@Frando
Copy link
Member Author

Frando commented Nov 29, 2023

why not store the client on the Entry?

That's what I did first. It makes the Entry generic over C: ServiceConnection<ProviderService>. And because it holds an RpcClient<C> it also cannot implement Deserialize. It would also make the LiveEvent generic over C and also not deserializable. So I thought to maybe stop making everything generic at that level, because LiveEvent<C> felt wrong. We could do two Entry structs, e.g. EntryClient and Entry or so, but to me the version with passing a client into the methods seems simpler. Open to other thoughts though!

@dignifiedquire
Copy link
Contributor

yeah that makes sense, this api is nice enough :)

@dignifiedquire dignifiedquire added this pull request to the merge queue Dec 12, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 12, 2023
@dignifiedquire dignifiedquire added this pull request to the merge queue Dec 12, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 12, 2023
@divagant-martian divagant-martian added this pull request to the merge queue Dec 12, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 12, 2023
@dignifiedquire dignifiedquire added this pull request to the merge queue Dec 12, 2023
Merged via the queue into main with commit 690e2aa Dec 12, 2023
27 checks passed
@dignifiedquire dignifiedquire deleted the refactor-client-entry branch November 28, 2024 10:11
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