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

Add documentation for preprocessors #6051

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

Conversation

jrhee17
Copy link
Contributor

@jrhee17 jrhee17 commented Dec 31, 2024

Notes

  • This PR serves as a design proposal for the new preprocessors API and will be merged once features regarding preprocessors are fully implemented.
  • See Poc/xds filter v2 jrhee17/armeria#36 for details on proposed changes

localhost_8000_docs_advanced-client-preprocessor

Copy link
Contributor

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

The new interface looks promising. 👍 👍 👍

@minwoox
Copy link
Contributor

minwoox commented Jan 2, 2025

I also understand that the decorator chain is mutable so that different decorators can be added in the preprocessor.
e.g. Different Retrying client based on the RouteConfiguration.

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

👍👍


## Implementing `HttpPreprocessor` and `RpcPreprocessor`

<type://HttpPreprocessor> and <type://RpcPreprocessor> expose a <type://PartialClientRequestContext>.
Copy link
Contributor

Choose a reason for hiding this comment

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

I can understand what Partial means, but it might be ambiguous for normal users.
What do you think of prefixing Pre to imply it is used in *Preprocessor?

Suggested change
<type://HttpPreprocessor> and <type://RpcPreprocessor> expose a <type://PartialClientRequestContext>.
<type://HttpPreprocessor> and <type://RpcPreprocessor> expose a <type://PreClientRequestContext>.

+-----------+ +-----------------+ +-----------------+ +-----------------+ +----------+ +-----------+
```

Note that unlike decorators, <typeplural://HttpPreprocessor> are not executed for RPC-based clients.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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

Successfully merging this pull request may close these issues.

3 participants