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

Did web http client #523

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

vdods
Copy link
Contributor

@vdods vdods commented Jul 28, 2023

This adds the HTTP client as discussed in #516

Note that this is a breaking change because of the semantics of the DIDWeb struct:

  • Currently, DIDWeb is an empty struct and can be instantiated as DIDWeb by itself (no {} needed), and appears in various places as &DIDWeb.
  • This change requires DIDWeb to be instantiated via methods DIDWeb::new_with_default_http_client (this is the drop-in replacement for existing functionality) or DIDWeb::new_with_http_client (if you want to specify your own HTTP client).

I altered the various call sites that were using &DIDWeb, and changed them to use an instance created with DIDWeb::new_with_default_http_client. However, codebases downstream of ssi that use DIDWeb will need to update their uses of DIDWeb (e.g. when constructing a DIDMethods) in order to integrate this change.

vdods added 2 commits July 28, 2023 14:02
…r resolution.

DIDWeb struct has an HTTP client to use for DID resolution.  It's incredibly slow to create a new
reqwest::Client, due to the overhead of loading the system's root certificates.  This HTTP client must
be specified by constructing the DIDWeb instance using new_with_default_http_client (to use defaults)
or DIDWeb::new_with_http_client if there is an specific reqwest::Client that should be reused.
Note that this is the recommended approach to using reqwest::Client (see
https://docs.rs/reqwest/latest/reqwest/struct.Client.html).
@CLAassistant
Copy link

CLAassistant commented Jul 23, 2024

CLA assistant check
All committers have signed the CLA.

@vdods
Copy link
Contributor Author

vdods commented Aug 20, 2024

Same deal here, I sent in a signed CLA Oct 6, 2021, but let me know if you need a new one for whatever reason.

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