Skip to content
This repository was archived by the owner on Apr 24, 2021. It is now read-only.

Feat: add WebFile File implementation.#26

Merged
Stebalien merged 2 commits intomasterfrom
feat/webfile
Aug 20, 2018
Merged

Feat: add WebFile File implementation.#26
Stebalien merged 2 commits intomasterfrom
feat/webfile

Conversation

@hsanjuan
Copy link
Copy Markdown
Contributor

A WebFile is a File which is read from a Web URL using a GET request.

This is used by cluster. I'd rather have it live here than in our code. The idea is that this allows to have ipfs add http://somehwere/something.jpg work rather easily.

@hsanjuan hsanjuan self-assigned this Aug 19, 2018
@hsanjuan hsanjuan requested review from Stebalien and keks August 19, 2018 17:24
@ghost ghost added the status/in-progress In progress label Aug 19, 2018
A WebFile is a File which is read from a Web URL using a GET request.
Copy link
Copy Markdown
Contributor

@keks keks left a comment

Choose a reason for hiding this comment

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

@hsanjuan This looks good in general. Do you know why Jenkins didn't build?

http.HandleFunc("/my/url/content.txt", func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintf(w, "Hello world!")
})
listener, err := net.Listen("tcp", ":18281")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would probably be cleaner to use https://godoc.org/net/http/httptest#NewServer for this, in case there already is something listening on that port.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah yeah, you're totally right, I always forget.

@Stebalien
Copy link
Copy Markdown
Member

This would also allow us to deprecate ipfs urlstore add so +1 from me.

@keks
Copy link
Copy Markdown
Contributor

keks commented Aug 20, 2018

That's great because that's the sharness test failing right now 😅

Edit: To clarify, it returns an unexpected error where there seems to occur none.

@hsanjuan
Copy link
Copy Markdown
Contributor Author

ipfs urlstore add

Had no idea this existed.

I don't know why jenkins fails, I brought it up with @victorbjelkholm . I have incorporated this to cluster and seems handy (and seems to work just fine).

@Stebalien
Copy link
Copy Markdown
Member

Had no idea this existed.

It's basically ipfs add --nocopy but for URLs. With this feature, we should be able to just use it with ipfs add --nocopy (maybe).

Copy link
Copy Markdown
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM. To use this in IPFS, we'd have to have some way to distinguish between URLs and paths but that's a separate issue.

We can ignore the broken CI for now...

@Stebalien Stebalien removed the status/in-progress In progress label Aug 20, 2018
@Stebalien Stebalien merged commit b5e40bc into master Aug 20, 2018
@Stebalien Stebalien deleted the feat/webfile branch August 20, 2018 23:53
@hsanjuan
Copy link
Copy Markdown
Contributor Author

Since URLs have paths, I just parse everything as url and check for existing http* schema.

The main usage I see is to pinbot.Add(http://catpicture.jpg), downloading and copying it, as a way to import content from the web (scraping it from it's potentially fragile web location).

@Stebalien
Copy link
Copy Markdown
Member

Stebalien commented Aug 21, 2018

The main usage I see is to pinbot.Add(http://catpicture.jpg), downloading and copying it, as a way to import content from the web (scraping it from it's potentially fragile web location).

I agree. Really, I'd also like to extend this to mirroring entire websites but that's a bit tricker (we'd probably have to add some kind of special "--mirror" flag).

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.

3 participants