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

chore(volo-http): add TimeoutLayer and FailOnStatus layer #522

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

yukiiiteru
Copy link
Member

Motivation

The previous MetaService was too coupled with the client configuration, it was very inelegant.

Solution

Remove MetaService and used TimeoutLayer and FailOnStatus layers to replace the previous MetaService with the two configs.

Break changes

  • ClientBuilder::fail_on_status has been removed, users should use FailOnStatus layer instead
  • ClientBuilder::set_request_timeout has been removed, users should use TimeoutLayer instead

Considering that Volo-HTTP is in rc version, there may be other break changes here later before we release a stable version.

TODO

  • Refactor Target, CallOpt and Config
  • Support setting timeout at request level

@yukiiiteru yukiiiteru requested review from a team as code owners November 6, 2024 07:18
Since the previous `MetaService` was too coupled with the client
configuration, it was very inelegant. So we removed `MetaService` and
used `TimeoutLayer` and `FailOnStatus` layers to replace the previous
`MetaService` with the two configs.

BREAK CHANGES:
- `ClientBuilder::fail_on_status` has been removed, users should use
  `FailOnStatus` layer instead
- `ClientBuilder::set_request_timeout` has been removed, users should
  use `TimeoutLayer` instead

TODO:
- Refactor `Target`, `CallOpt` and `Config`
- Support setting timeout at request level

Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
@yukiiiteru yukiiiteru merged commit 34f6a10 into cloudwego:main Nov 6, 2024
13 checks passed
@yukiiiteru yukiiiteru deleted the chore/remove-meta-service branch November 6, 2024 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants