-
Notifications
You must be signed in to change notification settings - Fork 57
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
allow overriding of DWN hosts #93
Conversation
Would rather call the property dwnEndpoints, as host makes it seem like an external entity is always involved, when it could simply be a static IP to a box in your own home. |
@csuwildcat yeah I agree - was struggling for a name. endpoints is also what DID spec calls them, will change. |
@csuwildcat how does this look? |
Lgtm |
@mistermoe were you going to push up some additions to the PR we discussed yesterday to make overriding other properties possible? Or should we start with |
i didn't get a chance to add those additional overrides we discussed yesterday. let's start with |
the only thing that gives me pause at the moment is that this Given that we haven't yet finished the first external agent, i think there's value in allowing people to provide custom dwn endpoints today. I think we could do it like so: const { web5, did } = await Web5.connect({
techPreview: {
dwnEndpoints: []
}
}); namespacing the |
Signed-off-by: Frank Hinek <[email protected]>
Spotted a couple issues with the existing PR:
both resolved with Commit 05e4e34 Still one remaining issue which is if with with Discussing with @mistermoe and @csuwildcat as to how we should handle. |
Signed-off-by: Frank Hinek <[email protected]>
Signed-off-by: Frank Hinek <[email protected]>
…1 endpoint Signed-off-by: Frank Hinek <[email protected]>
c6ddd50
to
b259d24
Compare
yeah calling connect with dwnEndpoints specified is in some ways assuming that is the special case that a did will be created for you, but wasn't sure where else it belongs in the short term. |
Co-authored-by: Frank Hinek <[email protected]> Co-authored-by: Moe Jangda <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥇
adds this enhancement: #86
Web5.connect()
now takes an options object which is documented here