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.

Sign up for GitHub

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

Jump to bottom

transport refactor update #4817

Merged
merged 2 commits into from
Jun 6, 2018
Merged

transport refactor update #4817

merged 2 commits into from
Jun 6, 2018

Conversation

Stebalien
Copy link
Member

@Stebalien Stebalien commented Mar 15, 2018 •
edited
Loading

Also:

  • update go-testutil
  • update the datastore interface (WIP)
  • update go-libp2p-record to remove useless DHT signatures

Blocked on:

  • Transport refactor.
  • Namesys republisher DHT fixes.

DO NOT MERGE

This PR will never be merged as-is, it's simply for review and testing.

libp2p/go-libp2p#297

@Stebalien Stebalien requested a review from Kubuxu as a code owner March 15, 2018 05:17
@ghost ghost assigned Stebalien Mar 15, 2018
@ghost ghost added the status/in-progress In progress label Mar 15, 2018
whyrusleeping
whyrusleeping reviewed Mar 17, 2018
View reviewed changes
core/builder.go Outdated
@@ -42,6 +44,10 @@ type BuildCfg struct {
// that will improve performance in long run
Permanent bool

// DisableEncryptedConnections disables connection encryption *entierly*.
Copy link
Member

Choose a reason for hiding this comment

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

typo. entierly

whyrusleeping
whyrusleeping reviewed Mar 17, 2018
View reviewed changes
core/commands/swarm.go
}
/*
// FIXME(steb):
swcon, ok := c.(*swarm.Conn)
Copy link
Member

Choose a reason for hiding this comment

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

definitely no need to reproduce the exact output here. But maybe we can print out the full transport.String() or something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, that won't include information about the muxer. Currently, we'd show:

<swarm.Conn[TCP] /local/maddr (<peer.ID local>) <-> /remote/maddr (<peer.ID remote>)>

(which should actually be cleaned up a bit...)

I've been toying with adding an Info function to transport conns (modeled after Context.Value):

func Info(key interface{}) interface{}

This would allow querying for arbitrary information.

whyrusleeping
whyrusleeping reviewed Mar 17, 2018
View reviewed changes
core/commands/swarm.go
@@ -384,14 +383,13 @@ ipfs swarm connect /ip4/104.131.131.82/tcp/4001/ipfs/QmaCpDMGvV2BGHeYERUEnRQAwe3
return
}

snet, ok := n.PeerHost.Network().(*swarm.Network)
// FIXME(steb): Nasty
swrm, ok := n.PeerHost.Network().(*swarm.Swarm)
Copy link
Member

Choose a reason for hiding this comment

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

ah, because we need to expose the backoff logic... It probably makes sense to just embed that information in the context.

Copy link
Member

Choose a reason for hiding this comment

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

ctx := context.WithValue(ctx, "swarmClearBackoff", true)
n.Peerhost.Connect(ctx, ...)

Copy link
Member

Choose a reason for hiding this comment

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

though, downside there is that it becomes much harder to notice when it stops working.

Copy link
Member Author

Choose a reason for hiding this comment

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

For now, I'm planning on leaving this as it is. This is more for my future self.

@Stebalien
Copy link
Member Author

I've been using this locally but haven't bothered fixing the gx deps.

@Kubuxu last review :). This should be a fairly short one (negative diffstat!).

Kubuxu
Kubuxu approved these changes Mar 29, 2018
View reviewed changes
Copy link
Member

@Kubuxu Kubuxu left a comment

Choose a reason for hiding this comment

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

Code seems good but something doesn't build.

@Stebalien
Copy link
Member Author

Stebalien commented Mar 29, 2018 •
edited
Loading

@Kubuxu I've pushed the gx dep update. Should hopefully build.

THIS PR IS NOT READY TO BE MERGED. It brings in an update to go-libp2p-kad-dht that needs #4753.

@Stebalien Stebalien force-pushed the feat/refactor branch 2 times, most recently from 581b634 to f900722 Compare March 29, 2018 21:04
@djdv djdv mentioned this pull request Apr 11, 2018
Merged
@Stebalien Stebalien force-pushed the feat/refactor branch 2 times, most recently from bc1d532 to ded46d0 Compare June 5, 2018 23:28
@Stebalien
Copy link
Member Author

This PR is pending a last-minute time-boxed review by @jbenet. This PR and it's dependencies will start to merge themselves (:man_shrugging:) tonight unless anyone objects (please don't).

💣

(you have been sufficiently warned)

@Stebalien Stebalien changed the title [WIP] transport refactor update transport refactor update Jun 6, 2018
@whyrusleeping
Copy link
Member

🙌 🎊 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

@Stebalien
transport refactor update
b84a71d 
License: MIT
Signed-off-by: Steven Allen <[email protected]>
@Stebalien
update gx imports
39f927a 
License: MIT
Signed-off-by: Steven Allen <[email protected]>
@Stebalien Stebalien added RFM and removed status/in-progress In progress labels Jun 6, 2018
@whyrusleeping
Copy link
Member

leeeeroyyyyyyyyyy jenkinsssssss!

@whyrusleeping whyrusleeping merged commit fc05376 into master Jun 6, 2018
@whyrusleeping whyrusleeping deleted the feat/refactor branch June 6, 2018 08:54
hacdias pushed a commit to ipfs/boxo that referenced this pull request Jan 27, 2023
@whyrusleeping
Merge pull request ipfs/kubo#4817 from ipfs/feat/refactor
c337728 
transport refactor update

This commit was moved from ipfs/kubo@fc05376
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whyrusleeping whyrusleeping whyrusleeping left review comments

@Kubuxu Kubuxu Kubuxu approved these changes

Assignees

@Stebalien Stebalien

Labels
RFM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Stebalien @whyrusleeping @Kubuxu