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

feat(server): Helia Docker #1

Merged
merged 25 commits into from
Oct 4, 2023
Merged

feat(server): Helia Docker #1

merged 25 commits into from
Oct 4, 2023

Conversation

whizzzkid
Copy link
Contributor

@whizzzkid whizzzkid commented Sep 28, 2023

Testing instructions:

  1. Make sure Docker 🐳 is running.
  2. Build the image: docker build . --tag helia
  3. Run the image: docker run -p 8080:8080 helia
  4. Open browser: http://localhost:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR
  5. Try IPNS: http://localhost:8080/ipns/docs.ipfs.tech/how-to/run-ipfs-inside-docker/#set-up

Output:

Screenshot 2023-10-03 at 11 55 57 PM
ipns-resolve-helia.mov

Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Copy link
Member

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

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

I don't see anything glaring, I'm good with this and had a few comments

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved

export const DEFAULT_MIME_TYPE = 'text/html'

const tests: Array<(input: testInput) => testOutput> = [
Copy link
Member

Choose a reason for hiding this comment

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

Might be useful to abstract the content-type tests to a type/interface and use that.

// testing file-type from buffer
async ({ bytes }): testOutput => (await fileTypeFromBuffer(bytes))?.mime,
// testing file-type from path
// ts-expect-error
Copy link
Member

Choose a reason for hiding this comment

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

why expect error? add more info why this is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mime.lookup complains about Unexpected value in conditional. A boolean expression is required.

Comment on lines 29 to 30
export async function parseContentType (input: testInput): Promise<string> {
let type = (await Promise.all(tests.map(async test => test(input)))).filter(Boolean)[0] as string
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this is much harder to read than it needs to be. Also, we're always calling each of the type check fns (map), and then looping over all of the successful ones (filter), instead of returning quickly on the first success.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

Comment on lines +62 to +67
const regex = /^\/(?<namespace>ip[fn]s)\/(?<address>[^/$]+)(?<relativePath>[^$]*)/
const result = path.match(regex)
if (result == null) {
throw new Error('Path is not valid, provide path as /ipfs/<cid> or /ipns/<path>')
}
return result.groups as { namespace: string, address: string, relativePath: string }
Copy link
Member

Choose a reason for hiding this comment

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

This is returning an error on a valid path:

http://localhost:8888/ipns/docs.ipfs.tech/how-to/run-ipfs-inside-docker/#set-up

see https://ipfs.io/ipns/docs.ipfs.tech/how-to/run-ipfs-inside-docker/#set-up

Copy link
Member

Choose a reason for hiding this comment

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

though I gave approval, this needs fixed (can be in later PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

const { Path } = await (await fetch(this.delegatedRoutingApi + address)).json()
this.ipnsResolutionCache.set(address, Path)
}
return this.fetch(`${this.ipnsResolutionCache.get(address)}${options?.path ?? ''}`)
Copy link
Member

Choose a reason for hiding this comment

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

this line is hard to follow tbh.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

whizzzkid and others added 3 commits October 4, 2023 13:31
Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Comment on lines +43 to +51
export async function parseContentType (input: testInput): Promise<string> {
for (const test of tests) {
const type = await test(input)
if (type !== undefined) {
return overrideContentType(type)
}
}
return DEFAULT_MIME_TYPE
}
Copy link
Member

Choose a reason for hiding this comment

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

👍

Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
@whizzzkid whizzzkid merged commit 13dd94b into main Oct 4, 2023
1 check passed
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