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

fix dht commands when pubsub routing is enabled #5200

Merged
merged 3 commits into from
Jul 9, 2018
Merged

Conversation

Stebalien
Copy link
Member

Instead of checking if Routing is a DHT (because it can now be a tiered router and still contain a DHT), stash the DHT in a separate field in the IPFS node (same as we do with the PSRouter).

I'd like to find a better way to fix this (hence the TODO) but it's tricky.

fixes #5197

This also fixes provider finding when the pubsub router is enabled: libp2p/go-libp2p-routing-helpers#3 (found by the new DHT sharness test).

See: libp2p/go-libp2p-routing-helpers#3

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien Stebalien requested a review from Kubuxu as a code owner July 7, 2018 06:42
@ghost ghost assigned Stebalien Jul 7, 2018
@ghost ghost added the status/in-progress In progress label Jul 7, 2018
@Stebalien Stebalien added need/review Needs a review and removed status/in-progress In progress labels Jul 7, 2018
@Stebalien Stebalien requested a review from a user July 7, 2018 21:01
res.SetError(ErrNotDHT, cmdkit.ErrNormal)
return
}

Copy link

Choose a reason for hiding this comment

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

This removed check covers checking for nil too, right? We don't seem to be checking for that anymore. (Same for the other places, if so.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. Fixed.

Instead of checking if Routing is a DHT (because it can now be a tiered
router and still contain a DHT), stash the DHT in a separate field in the IPFS
node (same as we do with the PSRouter).

fixes #5197

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@ghost ghost added the status/in-progress In progress label Jul 9, 2018
@Stebalien Stebalien requested a review from a user July 9, 2018 08:00
@whyrusleeping whyrusleeping merged commit 0461716 into master Jul 9, 2018
@ghost ghost removed the status/in-progress In progress label Jul 9, 2018
@Stebalien Stebalien deleted the fix/5197 branch July 9, 2018 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/review Needs a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: routing service is not a DHT (ipfs dht findprovs)
2 participants