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

Adding timeout overrides to NodeConnectionManager methods #363

Closed
tegefaulkes opened this issue Mar 25, 2022 · 6 comments · Fixed by #378
Closed

Adding timeout overrides to NodeConnectionManager methods #363

tegefaulkes opened this issue Mar 25, 2022 · 6 comments · Fixed by #378
Assignees
Labels
development Standard development r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices

Comments

@tegefaulkes
Copy link
Contributor

Specification

While we can specify the timeouts when creating a NodeConnectionManager there are cases where we want to be able to specify a timeout when creating a connection.

For this we will need to add an optional connConnectTime parameter to any NodeConnectionManager method that can create a connection. this ill override the timeout of trying to create a connection if set.

Additional context

Tasks

  1. Add optional timeouts to all of the NodeConnectionManager methods that could create a new connection.
@tegefaulkes tegefaulkes added the development Standard development label Mar 25, 2022
@tegefaulkes tegefaulkes self-assigned this Mar 25, 2022
@tegefaulkes
Copy link
Contributor Author

Didn't really need an issue for this since it's a small change. But it leaves a paper trail for discussion.

I'm in the process of adding the timeout to the methods such as createConnection, establishNodeConnection , withF and withG being the main ones. Do we want to add optional timeouts for getClosestGlobalNodes, getRemoteNodeClosestNodes, syncNodeGraph etc?

@CMCDragonkai

@CMCDragonkai
Copy link
Member

Some of those methods could an async generator right?

@tegefaulkes
Copy link
Contributor Author

tegefaulkes commented Mar 25, 2022

Yeah, withConnG is an async gen, the others are normal.

tegefaulkes added a commit that referenced this issue Mar 25, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
CMCDragonkai pushed a commit that referenced this issue Mar 26, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
CMCDragonkai pushed a commit that referenced this issue Mar 26, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
tegefaulkes added a commit that referenced this issue Mar 28, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
tegefaulkes added a commit that referenced this issue Mar 28, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
@tegefaulkes
Copy link
Contributor Author

I'm going to update this to use timers instead of a timeout value. this way we can compose timeouts.

@tegefaulkes tegefaulkes mentioned this issue Mar 28, 2022
29 tasks
tegefaulkes added a commit that referenced this issue Mar 28, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
@tegefaulkes
Copy link
Contributor Author

Fixed up the timeouts to use timers.

CMCDragonkai pushed a commit that referenced this issue Mar 29, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
tegefaulkes added a commit that referenced this issue Mar 29, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
tegefaulkes added a commit that referenced this issue Mar 30, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
tegefaulkes added a commit that referenced this issue Apr 8, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
@emmacasolin
Copy link
Contributor

The resolution of this issue should allow for #353 to be easily completed since it will allow for a timeout to be set for the NodeConnectionManager methods called within the Discovery domain.

tegefaulkes added a commit that referenced this issue Jun 1, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
tegefaulkes added a commit that referenced this issue Jun 1, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
tegefaulkes added a commit that referenced this issue Jun 2, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
tegefaulkes added a commit that referenced this issue Jun 6, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
tegefaulkes added a commit that referenced this issue Jun 7, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
tegefaulkes added a commit that referenced this issue Jun 10, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
emmacasolin pushed a commit that referenced this issue Jun 14, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
emmacasolin pushed a commit that referenced this issue Jun 14, 2022
In some cases we want to specify how long we attempt to connect to a node on a per-connection basis.

Related #363
@teebirdy teebirdy added the r&d:polykey:core activity 3 Peer to Peer Federated Hierarchy label Jul 24, 2022
@CMCDragonkai CMCDragonkai added r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices and removed r&d:polykey:core activity 3 Peer to Peer Federated Hierarchy labels Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices
Development

Successfully merging a pull request may close this issue.

4 participants