Skip to content
This repository has been archived by the owner on Mar 24, 2023. It is now read-only.

Update types with a clearer API, and fix compile issues #1

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

willscott
Copy link
Collaborator

  • Creates an explicit API for backing the HTTP module
  • Remove direct dependency of the ipfs core api and other concrete large dependencies
  • modifies directory listing behavior, to expose an interface for getting directory item sizes without requesting the full file data.

@willscott
Copy link
Collaborator Author

tagging @aschmahmann and @lidel to provide a heads up that i'm still slowly making progress on trying to get a functional pulled out version of this module of code.

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Cool! I won't have time to review until after Iceland but some quick thoughts:

@willscott
Copy link
Collaborator Author

  • this introduces a new API that will allow for use with Tsizes already
  • this is quite a bit more major of a refactor than that proposed in feat: make corehttp a reusable component ipfs/kubo#9070 - but the interface there will make it easier to test having kubo use this one instead through a shim, i think.

@lidel
Copy link
Member

lidel commented Jul 26, 2022

@willscott Finally got to this after IPFS Thing: this and other gateway extraction efforts (rainbow, Ian's PR) were discussed during Kubo standup today (ipfs/kubo#8524 (comment)), below is a longer summary that should be useful for you.

Gist: given current prioritization and limited resources, we will refine Gateway interface while code remains in ipfs/kubo repo, benefiting from existing tests, with intent of doing extraction as part of the wider "libipfs" efforts.

On API changes from this PR: Sounds like a good direction, but the diff is too big to evaluate without tests.

Suggestion: It is ok to submit a more bold API like one proposed here, but for us to review it you need to submit a PR similar to ipfs/kubo#9070

Additional takeaways / rationale:

  • we want to move forward with gateway extraction, but doing it here (separate repo, no shared history) makes it impossible to review in actionable way
    • git history is lost / diverged from Kubo codebase, hard to diff
    • Kubo has a comprehensible end-to-end suite in test/sharness/*gateway*.sh which makes refactors safer, and even that is not bulletproof, we catch and add new regression tests (example)
    • there are no end-to-end tests in this repo, so we can only casually comment on the API but don't feel confident enough to commit to switching Kubo to it
  • doing this in Kubo repo (like Ian's PR) is a better approach, maintenance-wise
    • there is no duplicated/diverged code, we can iterate on this across multiple Kubo releases
    • we benefit from end-to-end tests being added to Kubo's master
  • the end goal is to extract gateway code

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants