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

example(bindings): add async ConfigResolver #4477

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

Conversation

jmayclin
Copy link
Contributor

Description of changes:

Adds an example showing how to use the async ConfigResolver.

Call-outs:

I haven't decided whether this should be a self contained server, or whether it should just be two separate resolver's that can be used. I think I'm leaning towards the latter.

Testing:

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

Is this a refactor change? If so, how have you proved that the intended behavior hasn't changed?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Mar 27, 2024
@jmayclin jmayclin marked this pull request as ready for review May 23, 2024 03:24
Comment on lines +84 to +86
// let config_resolver = ConfigResolver::new(self.server_config(animal));
// ```
// because the compiler will complain that `&self` doesn't live long enough.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering: this doesn't work because ConfigResolver::new doesn't own self, right? Like, passing animal should be fine here because it's moved?

I'm curious if there are other ways to do this besides async move? Like, could a struct itself implement 'static + Send + Sync + Future<Output = Result<Config, Error>>> or something so that ConfigResolver::new could own the whole thing? Not sure if that question makes any sense.

AsyncAnimalConfigResolver { cert_directory }
}

// This method will lookup the appropriate certificates and read them from disc
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
// This method will lookup the appropriate certificates and read them from disc
// This method will lookup the appropriate certificates and read them from disk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants