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

refactor(app/test): minor tweaks to linkerd-app-test #3428

Merged
merged 7 commits into from
Dec 6, 2024

Conversation

cratelyn
Copy link
Collaborator

@cratelyn cratelyn commented Dec 5, 2024

while handling the upgrade to hyper 1.0, i noticed some small changes that'd be nice to make.

most importantly, with respect to linkerd/linkerd2#8733, this commit outlines http_util::http_request(). hyper 1.0 provides version specific SendRequest types for HTTP/1 and HTTP/2, so rather than try to be polymorphic across both senders, we send the request at the call site.

two other commits remove pub attributes for functions that aren't called externally. we'll address run_proxy() and connect_client() in a subsequent PR, because the dependent tests use both HTTP/1 and HTTP/2.

this is not used elsewhere. it can be private.

Signed-off-by: katelyn martin <[email protected]>
this function abstracts over one statement, at the cost of needing to
name the `SendRequest` type.

because hyper's 1.0 interface provides multiple `SendRequest` types
based on the HTTP version being used. to avoid needing to deal with
polymorphism, and to implicitly address a function-level allowance of
deprecated types, we remove this function and move this statement to the
function's call sites.

Signed-off-by: katelyn martin <[email protected]>
this function previously called `std::str::from_utf8`, before calling
`str::to_owned` on the result. because of this, there is a bit of extra
gymnastics, passing a `&body[..]` slice along after collecting the body
bytes.

this is both more baroque and less performant. this commit updates the
call, using `String::from_utf8` to collect the body, sparing a needless
`to_owned()`/`clone()` call.

Signed-off-by: katelyn martin <[email protected]>
the same `pub use` bundle is found in the parent `lib.rs`.

this removes it, and refers to the same `mod io {}` defined above.

Signed-off-by: katelyn martin <[email protected]>
@cratelyn cratelyn force-pushed the kate/hyper-1.x-app-test-refactors branch from c28c7ef to ebc3832 Compare December 5, 2024 21:06
@cratelyn cratelyn marked this pull request as ready for review December 5, 2024 21:34
@cratelyn cratelyn requested a review from a team as a code owner December 5, 2024 21:34
linkerd/app/inbound/src/http.rs Show resolved Hide resolved
#3428 (comment)

we can simplify these statements sending requests, by using
`ServiceExt::oneshot(..)`.

Signed-off-by: katelyn martin <[email protected]>
@cratelyn cratelyn merged commit a39eb30 into main Dec 6, 2024
15 checks passed
@cratelyn cratelyn deleted the kate/hyper-1.x-app-test-refactors branch December 6, 2024 17:14
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.

2 participants