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

Network Adapter Refactoring and Tests #90

Closed
wants to merge 135 commits into from
Closed

Network Adapter Refactoring and Tests #90

wants to merge 135 commits into from

Conversation

llSourcell
Copy link
Contributor

There were a few confusing naming conventions specifically with network adapter and network service that i clarified.

There was also some circular logic in the network adapter code that needed more clarification.
The NetAdapter interface now contains the HandleMessage method.

Network Adapter needed tests so I went ahead and implemented some tests. 2 of the tests will fail because they depend on a TODO method from Brian, so they are currently commented out.

jbenet and others added 30 commits September 17, 2014 20:34
fix(net:msg) use vendored imports
use a third-party pubsub library for internal communications

Insights:
* Within bitswap, the actors don't need anything more than simple pubsub
behavior. Wrapping and unwrapping messages proves unneccessary.

Changes:
* Simplifies the interface for both actors calling GetBlock and actors
receiving blocks on the network
* Leverages a well-tested third-party pubsub library

Design Goals:
* reduce complexity
* extract implementation details (wrapping and unwrapping data, etc)
from bitswap and let bitswap focus on composition of core algorithms
operations
@llSourcell llSourcell changed the title Network Adapter Refactoring and tests Network Adapter Refactoring and Tests Sep 18, 2014
@whyrusleeping
Copy link
Member

Youre gonna kill me with having your branch name different than the one you are requesting a pull from...

// NewSender wraps a network Service to perform translation between
// networkAdapter implements NetworkAdapter
type networkAdapter struct {
networkService NetService
Copy link
Member

Choose a reason for hiding this comment

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

this naming is inconsistent with the other changes you are making...

@whyrusleeping
Copy link
Member

Im confused about your reasoning for the naming changes

@llSourcell
Copy link
Contributor Author

Golint thought that calling a type like Network.networkAdapter is too repetitive, suggested i switch the name so i used net instead of network.

@btc
Copy link
Contributor

btc commented Sep 22, 2014

exchange/bitswap/network has matured a bit

see: https://github.com/jbenet/go-ipfs/tree/master/exchange/bitswap

@btc btc closed this Sep 22, 2014
@aschmahmann aschmahmann mentioned this pull request Sep 22, 2020
72 tasks
ribasushi pushed a commit that referenced this pull request Jul 4, 2021
Use multiple cores in logger
@aschmahmann aschmahmann mentioned this pull request Sep 30, 2021
62 tasks
ariescodescream pushed a commit to ariescodescream/go-ipfs that referenced this pull request Oct 23, 2021
fix: go1.9 monotonic time (for real this time)
@aschmahmann aschmahmann mentioned this pull request Dec 13, 2021
59 tasks
laurentsenta pushed a commit to laurentsenta/kubo that referenced this pull request Feb 25, 2022
feat: remove strict signing pubsub option.
laurentsenta pushed a commit to laurentsenta/kubo that referenced this pull request Feb 25, 2022
…ct-signing-option

feat: remove strict signing pubsub option.
laurentsenta pushed a commit to laurentsenta/kubo that referenced this pull request Mar 4, 2022
…ct-signing-option

feat: remove strict signing pubsub option.
laurentsenta pushed a commit to laurentsenta/kubo that referenced this pull request Mar 4, 2022
…ct-signing-option

feat: remove strict signing pubsub option.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants