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

Initialize CT log client once #350

Merged
merged 1 commit into from Jan 27, 2022
Merged

Conversation

nsmith5
Copy link
Contributor

@nsmith5 nsmith5 commented Jan 26, 2022

This work initializes the CT log client once and uses it for all requests instead of making a new one each time.

Thoughts for reviewers

The implementation here breaks from our current pattern of injecting dependencies by stuffing them into the request context. I think that explicit dependency inject makes it easier to reason about and test the code. This looks like explicitly asking for the API dependencies on construction and saving them in a struct so they can be accessed like that instead of pulling them out of the request context.

If ya'll agree that this pattern is more clear, I'm happy to iterate and do the same for the other API dependencies like the certificate authority backend and perhaps the logger. If not I'm happy to close this PR and fix using the context stuffing method instead.

Release Note

* Reuses the CT log client for all API requests instead of making a new client each time

Uses an explicit dependency injection instead of context stuffing
to add a CT log to the API handler.

Signed-off-by: Nathan Smith <nathan@nfsmith.ca>
@dlorenc dlorenc merged commit 7246b83 into sigstore:main Jan 27, 2022
@nsmith5 nsmith5 deleted the single-ct-log-client branch January 27, 2022 14:10
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.

None yet

2 participants