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

fullrt: fix default values not applied on config #764

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MichaelMure
Copy link
Contributor

It looks like an oversight.

Without this, FullRT doesn't act as a drop-in replacement for IpfsDHT. Worse, those default values can't be reached as they are internals.

@MichaelMure
Copy link
Contributor Author

cc @aschmahmann

@aschmahmann
Copy link
Contributor

Discussed offline, I think we should be moving away from having the defaults be for the IPFS Public DHT since there are already a few networks out there.

Also, the FullRT client is not a drop-in replacement for the IpfsDHT client (added some docs in #768), so the idea that the options aren't quite a drop-in replacement doesn't seem so bad.

If we think it's helpful to have something like an IpfsDHTOptions function within the fullrt package to make it easier for folks to discover the relevant options I'm game for that though.

WDYT?

@MichaelMure
Copy link
Contributor Author

In that case, maybe a completely different set of options would be better? The current situation is quite confusing as a user as you don't know which options are supported and if their behavior differ. Not operating on the same default makes it also quite error prone.

Additionally, there should be an identical way to configure the network properties (bucket size ...).

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.

2 participants