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

Support for ipfs add <url> #6065

Closed
NatoBoram opened this issue Mar 8, 2019 · 11 comments
Closed

Support for ipfs add <url> #6065

NatoBoram opened this issue Mar 8, 2019 · 11 comments
Labels
help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature

Comments

@NatoBoram
Copy link
Contributor

Version information:

go-ipfs version: 0.4.19-
Repo version: 7
System version: arm64/android
Golang version: go1.11.5

Type:

enhancement

Description:

I noticed that IPFS Cluster supported ipfs-cluster-ctl add <url>, but that feature isn't in IPFS. I think it's a pretty amazing command and I'd rather use that than urlstore.

Relevant : ipfs-cluster/ipfs-cluster#701 (comment)

@Stebalien Stebalien added the kind/enhancement A net-new feature or improvement to an existing feature label Mar 8, 2019
@Stebalien
Copy link
Member

Completely agree. Personally, I'd like to replace urlstore add with ipfs add --nocopy URL but that's a bit trickier.

FYI, if you'd like to implement this, it shouldn't be hard. We already have a "WebFile" file type (defined in go-ipfs-files). All we need to do is to modify the commands library (go-ipfs-cmds) to detect URLs passed as file arguments and use the "WebFile" type instead of the normal file type.

@Stebalien Stebalien added the help wanted Seeking public contribution on this issue label Mar 8, 2019
@Jorropo
Copy link
Contributor

Jorropo commented Mar 12, 2019

@Stebalien I will try for my first contrib here.

@jmank88
Copy link

jmank88 commented Mar 17, 2019

@Jorropo I've started hacking on this as well. Happy to collaborate if you want.

I've got go-ipfs-cmds/cli.Parse recognizing urls and using files.WebFile. It works (produces expected idential CID and is retrievable) but seems messy (unexpected parent paths added, https://->https:/) and doesn't work with --nocopy. Here is an example run with a fake url:

> ipfs add https://google.cloud.storage/v0/b/bucket/o/Qm...?alt=media&token=XXXXX
added Qm... https:/google.cloud.storage/v0/b/bucket/o/Qm...?alt=media
added Qm... https:/google.cloud.storage/v0/b/bucket/o
added Qm... https:/google.cloud.storage/v0/b/bucket
added Qm... https:/google.cloud.storage/v0/b
added Qm... https:/google.cloud.storage/v0
added Qm... https:/google.cloud.storage
added Qm... https:
> ipfs cat Qm...
hello world!

The url is recognized and a files.WebFile created (by cli.Parse), but then the content is streamed over multipart/form-data and http.parseRequest creates a files.multipartDirectory on the other end. What I'm really interested in is the --nocopy case, but I don't see an obvious way of sending the url over the http request for http.parseRequest to create it's own files.WebFile. @Stebalien Am I overlooking something simple? Or will the existing format need to be extended?

@Stebalien
Copy link
Member

With respect to the file paths, I'm not sure, I'd have to see the code.

For nocopy, we do actually (fow now) send the files over the HTTP API. We just also send the full path of the file in an abspath mime header. We then hash the data like usual except, instead of putting it into our local datastore, we throw it away and record a reference to the file from which it came.

We handle ipfs urlstore slightly differently. There, we send a URL as text (normal string argument) and download the file on the daemon.

The easiest path for ipfs add --nocopy URL would likely be to do what ipfs add --nocopy FILE does and just stick the URL in the abspath header. There'll be a slight performance hit but we have to download the file regardless (to hash it).

TL;DR: Make WebFile implement FileInfo.

@jmank88
Copy link

jmank88 commented Mar 18, 2019

TL;DR: Make WebFile implement FileInfo.

Great! And that should work for cluster-ctl automatically as well, no?

@Stebalien
Copy link
Member

Stebalien commented Mar 18, 2019

I believe so. Actually, I really don't know.

@Jorropo
Copy link
Contributor

Jorropo commented Mar 18, 2019

@jmank88 personaly I just copied the code from urlstore command to add and with an filestore.IsUrl test if that an url, if it is just fire the good one, I'm moving this code to an external file to not produce duplicate (wich is ironicaly very hard for me because I'm new with go).

@jmank88
Copy link

jmank88 commented Mar 21, 2019

@Stebalien second PR is up as a draft. Blocked waiting for release of go-ipfs-files v0.0.2.

@jmank88
Copy link

jmank88 commented Mar 22, 2019

@Stebalien Third is up. Blocked waiting for go-ipfs-cmds v0.0.4.

@Stebalien
Copy link
Member

All done and merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants