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

feat: adding relay (#181) #201

Closed
wants to merge 3 commits into from
Closed

feat: adding relay (#181) #201

wants to merge 3 commits into from

Conversation

daviddias
Copy link
Member

No description provided.

@daviddias daviddias mentioned this pull request Mar 27, 2017
53 tasks
@dryajov
Copy link
Member

dryajov commented Mar 28, 2017

Couldn't push to this branch, so here is a PR with the latest changes - #204

@dryajov
Copy link
Member

dryajov commented Apr 6, 2017

@diasdavid @dignifiedquire

This PR: #216 is already merged here because I need this functionality in circuit, but it is its own PR.

@daviddias
Copy link
Member Author

@dryajov mind squashing the commits on this branch? The commit history is a bit messy making it harder to review. We have the best practice of using rebase and not merge.

@dryajov
Copy link
Member

dryajov commented Apr 7, 2017

@diasdavid will do!

@dryajov dryajov force-pushed the feat/adding-relay branch 4 times, most recently from 04b4150 to 135526d Compare April 7, 2017 22:17
@dryajov dryajov self-assigned this Apr 7, 2017
@daviddias
Copy link
Member Author

@dryajov could you take this PR, #216 and #217 and coalesce it all in one single PR submitted by you, it will make the review way simpler for both of us :)

@dryajov
Copy link
Member

dryajov commented May 10, 2017

@diasdavid both #216 and #217 are already merged with this PR, we should be able to safely close those.

@@ -55,7 +57,7 @@ module.exports = function connection (swarm) {
}
const b58Str = peerInfo.id.toB58String()

swarm.muxedConns[b58Str] = { muxer: muxedConn }
swarm.muxedConns[b58Str] = {muxer: muxedConn}
Copy link
Member Author

Choose a reason for hiding this comment

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

unnecessary cosmetic change

@@ -92,6 +94,11 @@ module.exports = function connection (swarm) {
})
},

relay (config) {
swarm.relay = true
swarm.transport.add(Circuit.tag, new Circuit.Dialer(swarm, config))
Copy link
Member Author

Choose a reason for hiding this comment

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

I rather prefer to make relay specific and not a 'transport' like this. The transport interface will evolve and that doesn't necessarily need to trickle to relay.

On the dial.js, after trying all transports, then it should try relay to get that first conn.

src/index.js Outdated
const dial = require('./dial')
const protocolMuxer = require('./protocol-muxer')
const plaintext = require('./plaintext')
const assert = require('assert')

const DEFAULT_TRANSPORT_PRIORITY = 1
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be removed now.

@daviddias
Copy link
Member Author

As I mentioned here:

The relay listener never gets used.

@dryajov dryajov force-pushed the feat/adding-relay branch 3 times, most recently from 389b73b to 4326ea6 Compare July 9, 2017 22:26
@dryajov
Copy link
Member

dryajov commented Aug 2, 2017

@diasdavid since we're not doing multihop dialing anymore and this was needed specifically to support it, should we close this PR?

@diasdavid We still need this for listening on specific relays, overall I think this makes sense as it allows extending and plugin with the swarm more easily.

feat: adding default priority for transports

feat: limit dialer cb should be called only once

feat: push circuit to be the last transport dialed

misc: rename relay method to enableRelayDialing

style: revert formatting
@dryajov dryajov requested review from a team and removed request for a team August 3, 2017 05:47
@dryajov
Copy link
Member

dryajov commented Aug 3, 2017

@diasdavid @lgierth @dignifiedquire Closing this since we're not doing multihop dialing anymore.

@dryajov dryajov closed this Aug 3, 2017
@daviddias daviddias deleted the feat/adding-relay branch August 3, 2017 20:29
@dryajov dryajov mentioned this pull request Aug 16, 2017
48 tasks
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.

2 participants