Skip to content
This repository has been archived by the owner on Dec 7, 2019. It is now read-only.

expose SetDeadline methods on streams #10

Closed
whyrusleeping opened this issue Aug 18, 2016 · 12 comments
Closed

expose SetDeadline methods on streams #10

whyrusleeping opened this issue Aug 18, 2016 · 12 comments

Comments

@whyrusleeping
Copy link
Collaborator

Most (if not all) of the stream multiplexers we have already have support for these methods on their streams. We should expose that down to the users

@jbenet
Copy link
Collaborator

jbenet commented Aug 19, 2016

This may be a libp2p decision. Maybe libp2p should use an abstraction like
context or proc natively. To express to users that it's really important to
cancel
On Thu, Aug 18, 2016 at 18:54 Jeromy Johnson notifications@github.com
wrote:

Most (if not all) of the stream multiplexers we have already have support
for these methods on their streams. We should expose that down to the users


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#10, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIcoY09Pxkj-QxCI8b3if9V4YYZz_QYks5qhOKvgaJpZM4Jn_r_
.

@jbenet
Copy link
Collaborator

jbenet commented Aug 19, 2016

cc @diasdavid
On Thu, Aug 18, 2016 at 22:11 Juan Benet juan2@benet.ai wrote:

This may be a libp2p decision. Maybe libp2p should use an abstraction like
context or proc natively. To express to users that it's really important to
cancel
On Thu, Aug 18, 2016 at 18:54 Jeromy Johnson notifications@github.com
wrote:

Most (if not all) of the stream multiplexers we have already have support
for these methods on their streams. We should expose that down to the users


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#10, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIcoY09Pxkj-QxCI8b3if9V4YYZz_QYks5qhOKvgaJpZM4Jn_r_
.

@daviddias
Copy link
Member

If adopted, it should be part of https://github.com/libp2p/interface-stream-muxer and set as an expectation of an implementation.

I'm not sure if I understand what is setting a deadline though, checked the code and I see that option, but when is it used?

@whyrusleeping
Copy link
Collaborator Author

@diasdavid its a common socket options, check the docs here: https://golang.org/pkg/net/#Conn

@daviddias
Copy link
Member

@whyrusleeping got it, when do you use it?

@jbenet
Copy link
Collaborator

jbenet commented Aug 21, 2016

@diasdavid it's effectively a timeout, when asking to send, listen, dial, etc, but only for a given amount of time.

@whyrusleeping i would much prefer to use a context abstraction in libp2p, across languages. contexts can give you deadlines, and cancellables, etc.

@hsanjuan
Copy link

While I mostly agree with @jbenet last comment, this might be the only thing breaking go-libp2p/master:

../../../go-peerstream/stream.go:96: s.smuxStream.SetDeadline undefined (type streammux.Stream has no field or method SetDeadline)
../../../go-peerstream/stream.go:100: s.smuxStream.SetReadDeadline undefined (type streammux.Stream has no field or method SetReadDeadline)
../../../go-peerstream/stream.go:104: s.smuxStream.SetWriteDeadline undefined (type streammux.Stream has no field or method SetWriteDeadline)

And it has been recently bubbled to the Stream interface anyway https://godoc.org/github.com/libp2p/go-libp2p-net#Stream

If I attempt a fix, does it stand a chance to be accepted? Or are going to go down the road of undoing libp2p/go-libp2p-net@f657968 (go-peerstream, go-libp2p-swarm at least are affected)

@hsanjuan
Copy link

@whyrusleeping I see some work being done here too that would need to be consolidated: https://github.com/whyrusleeping/go-stream-muxer/commits/master

@victorb
Copy link
Member

victorb commented Jan 23, 2017

I'm trying to run the tests in this project and this is my adventure-log from it:

➜  ~ go get -d github.com/jbenet/go-stream-muxer

➜  go-stream-muxer git:(master) cd $GOPATH/src/github.com/jbenet/go-stream-muxer

➜  go-stream-muxer git:(master) make deps
go get github.com/whyrusleeping/gx
go get github.com/whyrusleeping/gx-go
gx --verbose install --global
installing package: go-stream-muxer-1.1.0
successfully found all deps for go-stream-muxer
installation of dep go-stream-muxer complete!
gx-go rewrite

➜  go-stream-muxer git:(master) make test
go get github.com/whyrusleeping/gx
go get github.com/whyrusleeping/gx-go
gx --verbose install --global
installing package: go-stream-muxer-1.1.0
successfully found all deps for go-stream-muxer
installation of dep go-stream-muxer complete!
gx-go rewrite
go test ./...
# github.com/jbenet/go-stream-muxer/suite
suite/suite_test.go:9:2: cannot find package "github.com/whyrusleeping/go-smux-spdystream" in any of:
	/usr/local/Cellar/go/1.7.4_2/libexec/src/github.com/whyrusleeping/go-smux-spdystream (from $GOROOT)
	/Users/user/go/src/github.com/whyrusleeping/go-smux-spdystream (from $GOPATH)
FAIL	github.com/jbenet/go-stream-muxer/suite [setup failed]
?   	github.com/jbenet/go-stream-muxer	[no test files]
?   	github.com/jbenet/go-stream-muxer/test	[no test files]
make: *** [test] Error 1

➜  go-stream-muxer git:(master) go get github.com/whyrusleeping/go-smux-spdystream

➜  go-stream-muxer git:(master) make test
go get github.com/whyrusleeping/gx
go get github.com/whyrusleeping/gx-go
gx --verbose install --global
installing package: go-stream-muxer-1.1.0
successfully found all deps for go-stream-muxer
installation of dep go-stream-muxer complete!
gx-go rewrite
go test ./...
# github.com/jbenet/go-stream-muxer/suite
suite/suite_test.go:10:2: cannot find package "github.com/whyrusleeping/go-smux-yamux" in any of:
	/usr/local/Cellar/go/1.7.4_2/libexec/src/github.com/whyrusleeping/go-smux-yamux (from $GOROOT)
	/Users/user/go/src/github.com/whyrusleeping/go-smux-yamux (from $GOPATH)
FAIL	github.com/jbenet/go-stream-muxer/suite [setup failed]
?   	github.com/jbenet/go-stream-muxer	[no test files]
?   	github.com/jbenet/go-stream-muxer/test	[no test files]
make: *** [test] Error 1

➜  go-stream-muxer git:(master) go get -v github.com/whyrusleeping/go-smux-yamux
github.com/whyrusleeping/go-smux-yamux (download)
github.com/hashicorp/yamux (download)
github.com/hashicorp/yamux
github.com/whyrusleeping/go-smux-yamux

➜  go-stream-muxer git:(master) make test
go get github.com/whyrusleeping/gx
go get github.com/whyrusleeping/gx-go
gx --verbose install --global
installing package: go-stream-muxer-1.1.0
successfully found all deps for go-stream-muxer
installation of dep go-stream-muxer complete!
gx-go rewrite
go test ./...
?   	github.com/jbenet/go-stream-muxer	[no test files]
# github.com/whyrusleeping/go-smux-muxado
../../whyrusleeping/go-smux-muxado/muxado.go:62: cannot use stream literal (type *stream) as type streammux.Stream in return argument:
	*stream does not implement streammux.Stream (missing SetDeadline method)
../../whyrusleeping/go-smux-muxado/muxado.go:71: cannot use stream literal (type *stream) as type streammux.Stream in return argument:
	*stream does not implement streammux.Stream (missing SetDeadline method)
FAIL	github.com/jbenet/go-stream-muxer/suite [build failed]
?   	github.com/jbenet/go-stream-muxer/test	[no test files]
make: *** [test] Error 2

In the end, seems *stream is missing SetDeadline method, which means I cannot run the tests...

@daviddias
Copy link
Member

daviddias commented Jan 23, 2017

@victorbjelkholm try skipping muxado (not important for now).

@Stebalien
Copy link
Member

I believe this is done. @whyrusleeping?

@whyrusleeping
Copy link
Collaborator Author

whyrusleeping commented Aug 31, 2017 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants