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.

Sign up for GitHub

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

Jump to bottom

Make gateway POST handle multipart file & pin flag #2205

Closed
wants to merge 2 commits into from
Closed

Make gateway POST handle multipart file & pin flag #2205

wants to merge 2 commits into from

Conversation

rht
Copy link
Contributor

@rht rht commented Jan 15, 2016

@rht rht mentioned this pull request Jan 15, 2016
Closed
5 tasks
@prusnak prusnak mentioned this pull request Jan 18, 2016
Closed
@rht rht force-pushed the gateway-multipart-add branch 2 times, most recently from a471686 to 666b755 Compare January 20, 2016 16:25
@rht
Copy link
Contributor Author

rht commented Jan 20, 2016

Test fixed. rht@666b755#diff-cbe32203deec9b995950e3110e5db668R69 is an example of adding an array of files (could be in the form of file[]=..., and can be nested).

Several apis to look at, in the order of decreasing features:

None of them support upload of multiple files in a single POST request.
The aws cli, however, has the sync command for folders.
What would be the reason for supporting file arrays?

imo, further dedup with ipfs add should eventually be done. Currently, the remaining differences with ipfs add are the params (Hidden, Trickle, Wrap, Chunker), and that the latter has a chan to output the hash of each files.

@rht rht force-pushed the gateway-multipart-add branch 3 times, most recently from 354b964 to 7c413fb Compare January 20, 2016 19:13
rht
rht reviewed Jan 20, 2016
View reviewed changes
commands/files/multipartfile.go
@@ -55,6 +55,22 @@ func NewFileFromPart(part *multipart.Part) (File, error) {
return f, nil
}

func NewFileFromRequest(r *http.Request) (File, error) {
contentType := r.Header.Get(contentTypeHeader)
mediaType, _, _ := mime.ParseMediaType(contentType)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

err ignored due to ce49541. If there is a more semantic way to express that this is run only when files are involved, including the statement used to state this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

redirect: #2226

Copy link
Member

Choose a reason for hiding this comment

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

i dont understand this statement.

@rht rht mentioned this pull request Jan 21, 2016
Closed
@rht rht added RFCR topic/gateway Topic gateway labels Jan 21, 2016
@jbenet
Copy link
Member

jbenet commented Jan 24, 2016

odd, travis didn't run... @rht can you rebase + push again so travis runs? thanks

jbenet
jbenet reviewed Jan 24, 2016
View reviewed changes
core/corehttp/gateway_handler.go
return "", err
}

dopin := r.FormValue("pin")
Copy link
Member

Choose a reason for hiding this comment

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

shouldnt use the form value, this check should check the command option.

Copy link
Member

Choose a reason for hiding this comment

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

ah nvm there is no command, it's gateway

@rht
Make gateway POST handle multipart file & pin flag
d6d0a2b 
License: MIT
Signed-off-by: rht <rhtbot@gmail.com>
@rht
Copy link
Contributor Author

rht commented Jan 25, 2016

Rebased.

@whyrusleeping
Copy link
Member

so, just to make sure i understand the goal here. You're wanting to use the unixfs fileAdder for writeable gateway code?

@rht
Copy link
Contributor Author

rht commented Jan 28, 2016

Yes, this dedup has been intended since #1543.

whyrusleeping
whyrusleeping reviewed Jan 29, 2016
View reviewed changes
core/corehttp/gateway_handler.go
@@ -301,21 +303,57 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
}

func (i *gatewayHandler) postHandler(w http.ResponseWriter, r *http.Request) {
nd, err := i.newDagFromReader(r.Body)
fileAdder, err := coreunix.NewAdder(i.node.Context(), i.node, nil)
Copy link
Member

Choose a reason for hiding this comment

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

we should use the closenotify code like in the get or head requests to make sure we don't have lingering uncanceled contexts from here.

@whyrusleeping
Copy link
Member

just one context thing and then LGTM.

@rht
Set closenotifer to all gateway requests
913a15e 
License: MIT
Signed-off-by: rht <rhtbot@gmail.com>
@rht
Copy link
Contributor Author

rht commented Jan 30, 2016

Added the context closenotifer to all gateway requests.

@rht rht mentioned this pull request Feb 2, 2016
Closed
@Kubuxu Kubuxu added need/review Needs a review and removed RFCR labels May 15, 2016
@rht rht closed this Jun 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
need/review Needs a review topic/gateway Topic gateway
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

gateway: file upload functionality?
4 participants
@rht @jbenet @whyrusleeping @Kubuxu