Skip to content

docs: Update Async Context docs #3229

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

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

Conversation

bryce-anderson
Copy link
Contributor

Motivation:

CapturedContext is not well represented in the docs now.

Modifications:

Adjust our documentation to descrive CapturedContext and its use cases.

Motivation:

CapturedContext is not well represented in the docs now.

Modifications:

Adjust our documentation to descrive CapturedContext and its
use cases.
== `AsyncContext`

Although `AsyncContext` may be convenient to use it shouldn't be overused when traditional argument passing is an
option. The intended use case of `AsyncContext` is to propagate context across method boundaries which do not
Copy link
Contributor

Choose a reason for hiding this comment

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

also maybe clarify between method and filter boundaries (just clarifying since I assume a big use case is across filters)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to API boundaries.

I'm not sure if filters are generally a good use case since our request and response methods carry a ContextMap themselves. Within the filter chain, this would be my preferred way to store the data since it's very tightly connected to the request and response objects it will be associated with.

@bryce-anderson bryce-anderson requested a review from daschl April 11, 2025 18:50
Copy link
Member

@idelpivnitskiy idelpivnitskiy left a comment

Choose a reason for hiding this comment

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

Overall LGTM, some minor comments:

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.

3 participants