Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

remove the DialTimeout #230

Merged
merged 1 commit into from
Dec 20, 2021
Merged

remove the DialTimeout #230

merged 1 commit into from
Dec 20, 2021

Conversation

marten-seemann
Copy link
Contributor

The timeout should be a configuration option for the swarm, not a global variable.

Related change in the swarm: libp2p/go-libp2p-swarm#302.

We should probably do the same for the AcceptTimeout.

The timeout should be a configuration option for the swarm, not a global
variable.
@github-actions
Copy link

gocompat says:

Branch 'master' set up to track remote branch 'master' from 'origin'.
"github.com/libp2p/go-libp2p-core/transport".DialTimeout TopLevelDeclarationDeleted

Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

wait, dont transports themselves use that?

@marten-seemann
Copy link
Contributor Author

wait, dont transports themselves use that?

Nope.

TCP uses an option: https://github.com/libp2p/go-tcp-transport/blob/1c17791c35777091afcb56a0707503bf82532a62/tcp.go#L98-L103
QUIC just uses the quic-go defaults.

@marten-seemann
Copy link
Contributor Author

Confirmed by building go-ipfs using this PR and libp2p/go-libp2p-swarm#302.

Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

well ok, @Stebalien was this ever useful?

Copy link
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.

I think we ended up using custom timeouts in transports, because the actual timeout is transport dependent.

@marten-seemann marten-seemann merged commit 0a19838 into master Dec 20, 2021
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.

None yet

3 participants