Conversation
| // the hour is a hard fallback, we don't expect it to happen, but just in case | ||
| defer cancel() | ||
|
|
||
| if cn, ok := w.(http.CloseNotifier); ok { |
There was a problem hiding this comment.
why are we removing the close notify stuff?
There was a problem hiding this comment.
We're wiring this to net/http.Request.Context() now (line 70)
There was a problem hiding this comment.
I dont think that context implicitly has the close notify stuff on it...
There was a problem hiding this comment.
Mh you're right. sloppy. I'm gonna move it into ServeHTTP() though so it also applies to POST/PUT ok?
There was a problem hiding this comment.
I'm gonna move it into ServeHTTP() though so it also applies to POST/PUT ok?
... in the next PR. gonna reinstate it here for now
There was a problem hiding this comment.
... in the next PR
haha, sounds good to me.
|
The interfaces look pretty good to me. Simple enough to be useful all over. @diasdavid could you take a look at it from the perspective of a 'core interface'? |
|
Okay I updated this to improve the context wiring. The close-notify stuff is reinstated, and apiOption is gone in favor of passing contexts into the coreapi functions, in order to satisfy this rule:
|
0e042b5 to
b7a1b66
Compare
License: MIT Signed-off-by: Lars Gierth <larsg@systemli.org>
License: MIT Signed-off-by: Lars Gierth <larsg@systemli.org>
|
I'm closing this in favor of #3244 which has been based on this PR here. |
Okay then, this still misses a few tests but is otherwise good for review. This uses the planned extraction of the gateway as a welcome excuse to start working on the Go side of Core API things. We're implementing the unspecified UnixfsAPI here, for now
Cat()andLs(). Near-future pull requests will implement more of UnixfsAPI (e.g.Add()), and of the Object API (i.e.SetData(),AddLink(),RmLink()).core/coreapi/interface-- will eventually move to the interface-ipfs-core repo, when UnixfsAPI gets specced (out of scope right now)core/coreapi-- holds the actual implementation. There's kind of a weird construct around context wiring, for the purpose of being able to passnet/http.Request.Context().cc @kevina @Kubuxu @whyrusleeping for review.