Skip to content

Implement a File Link Resolver #5981

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

Open
wants to merge 8 commits into
base: krishna/refactor-main
Choose a base branch
from

Conversation

incrypto32
Copy link
Member

This PR implements a file system based link resolver aimed to replace IpfsLinkResolver for local development, this is in preparation for #5977

@incrypto32 incrypto32 changed the base branch from master to krishna/refactor-main May 1, 2025 13:02
@incrypto32 incrypto32 self-assigned this May 1, 2025
@lutter lutter self-requested a review May 1, 2025 15:54
@incrypto32 incrypto32 force-pushed the krishna/refactor-main branch from c8b8169 to 15fe3c3 Compare May 6, 2025 10:38
@incrypto32 incrypto32 force-pushed the krishna/file-link-resolver branch from 3ef16d5 to 23e91f1 Compare May 6, 2025 12:59

async fn json_stream(&self, _logger: &Logger, _link: &Link) -> Result<JsonValueStream, Error> {
Err(anyhow!("json_stream is not implemented for FileLinkResolver").into())
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Making these always return errors is sorta ugly. It would be nicer to have a trait FileResolver that doesn't have these two and a trait LinkResolver that does and change the code to use the right one. That's going to be a bigger change, so fine to do it in a separate PR, but we shouldn't let this linger for too long.

The danger with leaving this too long is that the code becomes brittle since now every user of LinkResolver needs to make sure it gets the right kind as that's not guaranteed by the type alone anymore

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