chore(app/inbound): address server::conn::Http deprecations
#3432
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
this was not being flagged as an unused variable, due to the `#[instrument]` attribute. (😉 _it's used as a field in the generated span!_) `connect_timeout(..)` doesn't use its parameter. to address some deprecations, and avoid the need for polymorphism / refactoring related to http/1 and http/2 connections being represented as distinct types in the hyper 1.0 api, we remove it. Signed-off-by: katelyn martin <[email protected]>
this addresses hyper 1.0 deprecations in the server side of the inbound proxy's http unit test suite logic. see <linkerd/linkerd2#8733> for more information. the client end of this change ends up being slightly involved, due to changes that will need to be made in `linkerd_app_test::http_util`. accordingly, those deprecations will be addressed in a subsequent commit. Signed-off-by: katelyn martin <[email protected]>
cratelyn
commented
Dec 6, 2024
Comment on lines
-678
to
+670
| #[allow(deprecated)] // linkerd/linkerd2#8733 | ||
| fn connect_timeout( | ||
| http: hyper::server::conn::Http, | ||
| ) -> Box<dyn FnMut(Remote<ServerAddr>) -> ConnectFuture + Send> { | ||
| fn connect_timeout() -> Box<dyn FnMut(Remote<ServerAddr>) -> ConnectFuture + Send> { |
There was a problem hiding this comment.
like #3421, this is another exercise in dodging deprecation work by... removing things! 🙂
cratelyn
commented
Dec 6, 2024
Comment on lines
-611
to
+598
| http: hyper::server::conn::Http, | ||
| server: hyper::server::conn::http1::Builder, |
Comment on lines
-633
to
+621
| http: hyper::server::conn::Http, | ||
| server: hyper::server::conn::http2::Builder<TracingExecutor>, |
There was a problem hiding this comment.
...and this, are the other core of this branch. we can tweak these types, without having to do any additional refactoring.
i'm punting the client end of this change into a separate branch, as mentioned in the description, because we currently share common helper functions for the http/1 and http/2 clients.
|
auto-merge enabled! 🤖 |
olix0r
approved these changes
Dec 9, 2024
cratelyn
added a commit
that referenced
this pull request
Dec 9, 2024
this commit changes the `connect_and_accept(..)` helper function used in
the proxy's unit tests, altering its logic such that it now runs
background tasks inside of a `JoinSet`.
then, subsequent commits hoist code out of `run_proxy()` and `connect_client()`, and consolidate our `async move {}` blocks. this both simplifies the control flow of this test infrastructure, but also addresses some more hyper deprecations.
for more information about our progressing upgrade to hyper 1.0, see linkerd/linkerd2#8733.
NB: this branch is based upon #3432.
---
* refactor(app/inbound): remove unused `Http` parameter
this was not being flagged as an unused variable, due to the
`#[instrument]` attribute. (😉 _it's used as a field in the generated
span!_)
`connect_timeout(..)` doesn't use its parameter.
to address some deprecations, and avoid the need for polymorphism /
refactoring related to http/1 and http/2 connections being represented
as distinct types in the hyper 1.0 api, we remove it.
Signed-off-by: katelyn martin <[email protected]>
* chore(app/inbound): address `server::conn::Http` deprecations
this addresses hyper 1.0 deprecations in the server side of the inbound
proxy's http unit test suite logic.
see <linkerd/linkerd2#8733> for more
information.
the client end of this change ends up being slightly involved, due to
changes that will need to be made in `linkerd_app_test::http_util`.
accordingly, those deprecations will be addressed in a subsequent
commit.
Signed-off-by: katelyn martin <[email protected]>
* refactor(app/test): reorder and document test helpers
this moves the public `connect_client(..)` function up to the top of the
group of functions, and adds a documentation comment explaining its
purpose.
a todo comment is left noting that this can be refactored to use tokio's
new `JoinSet` type.
Signed-off-by: katelyn martin <[email protected]>
* refactor(app/test): briefly defer `spawn()`ing tasks
this is a small tweak, defering the call to `tokio::spawn(..)` to make
using `JoinSet` easier.
Signed-off-by: katelyn martin <[email protected]>
* refactor(app/test): use `JoinSet` for background tasks
this commit changes the `connect_and_accept(..)` helper function used in
the proxy's unit tests, altering its logic such that it now runs
background tasks inside of a `JoinSet`.
Signed-off-by: katelyn martin <[email protected]>
* refactor(app/test): tweak proxy process
this commit makes two minor changes to the `run_proxy()` helper
function:
* remove a frivolous `.map(|_| ())` on a result that already had a tuple
`Ok` type.
* use `ServiceExt::oneshot` for brevity.
this should have no effect on the tests, we're just golfing things down
a bit.
Signed-off-by: katelyn martin <[email protected]>
* refactor(app/test): hoist `instrument(..)` calls
Signed-off-by: katelyn martin <[email protected]>
* refactor(app/test): remove `run_proxy(..)`
we've nudged this along well enough that this function can now
reasonably be removed.
Signed-off-by: katelyn martin <[email protected]>
* refactor(app/test): remove `connect_client(..)`
and once more, we remove a helper function that isn't doing quite so
much work, and whose signature contains deprecated hyper 1.0 types.
Signed-off-by: katelyn martin <[email protected]>
* refactor(app/test): consolidate anonymous futures
we create `async move {}` blocks in multiple places, without any clear
benefit.
this commit consolidates them into one place: when we spawn a task onto
the `JoinSet`.
Signed-off-by: katelyn martin <[email protected]>
---------
Signed-off-by: katelyn martin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this addresses hyper 1.0 deprecations in the server side of the inbound
proxy's http unit test suite logic.
see linkerd/linkerd2#8733 for more
information.
the client end of this change ends up being slightly involved, due to
changes that will need to be made in
linkerd_app_test::http_util.accordingly, those deprecations will be addressed in a subsequent
commit.