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.

Sign up for GitHub

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

Jump to bottom

http2::handshake() is useless in 1.0 given no default executor #3128

Closed
conradludgate opened this issue Jan 25, 2023 · 1 comment · Fixed by #3135
Closed

http2::handshake() is useless in 1.0 given no default executor #3128

conradludgate opened this issue Jan 25, 2023 · 1 comment · Fixed by #3135
Labels
B-breaking-change Blocked: this is an "API breaking change". C-bug Category: bug. Something is wrong. This is bad!
Milestone
1.0 RC3

Comments

@conradludgate
Copy link

conradludgate commented Jan 25, 2023 •
edited
Loading

Version

hyper 1.0.0-rc2

Description

hyper::client::conn::http2::handshake(io).await causes an instant panic. Internally it performs Builder::new().handshake(io).await which uses the Exec::Default executor. There is no longer a default tokio runtime implied since the removal of the runtime feature.

https://github.com/hyperium/hyper/blob/master/src/common/exec.rs#L36-L38

There is a TODO in this file to refactor executors. But I couldn't find a relevant issue, so I wanted to open one to ensure this doesn't get included in the official release
@conradludgate conradludgate added the C-bug Category: bug. Something is wrong. This is bad! label Jan 25, 2023
@conradludgate conradludgate changed the title handshake() helper methods are useless in 1.0 given no default executor http2 handshake() helper methods are useless in 1.0 given no default executor Jan 25, 2023
@conradludgate conradludgate changed the title http2 handshake() helper methods are useless in 1.0 given no default executor http2::handshake() is useless in 1.0 given no default executor Jan 25, 2023
@seanmonstar
Copy link
Member

Good catch, thank you!

@seanmonstar seanmonstar added this to the 1.0 RC3 milestone Jan 26, 2023
@seanmonstar seanmonstar added the B-breaking-change Blocked: this is an "API breaking change". label Jan 26, 2023
@dswij dswij mentioned this issue Jan 30, 2023
Merged
seanmonstar pushed a commit that referenced this issue Jan 31, 2023
@dswij
feat(client): http2 builder now requires an Executor (#3135)
8068aa0 
This commit removes `common::Exec::Default` that just panics when used. We are
removing `tokio`, leaving `Exec::Default` with no implementation and
panics when `Exec::execute` is called.

Since `Exec::Default` has no purpose, it is being removed and user
should now provide an implementation of executor.

Closes #3128 

BREAKING CHANGE:  `hyper::client::conn::Http2::Builder::new` now requires an executor argument.
@github-project-automation github-project-automation bot moved this from Todo to Done in hyper 1.0 Jan 31, 2023
0xE282B0 pushed a commit to 0xE282B0/hyper that referenced this issue Jan 12, 2024
@dswij @0xE282B0
feat(client): http2 builder now requires an Executor (hyperium#3135)
42b20a1 
This commit removes `common::Exec::Default` that just panics when used. We are
removing `tokio`, leaving `Exec::Default` with no implementation and
panics when `Exec::execute` is called.

Since `Exec::Default` has no purpose, it is being removed and user
should now provide an implementation of executor.

Closes hyperium#3128 

BREAKING CHANGE:  `hyper::client::conn::Http2::Builder::new` now requires an executor argument.
0xE282B0 pushed a commit to 0xE282B0/hyper that referenced this issue Jan 16, 2024
@dswij @0xE282B0
feat(client): http2 builder now requires an Executor (hyperium#3135)
3b76f3f 
This commit removes `common::Exec::Default` that just panics when used. We are
removing `tokio`, leaving `Exec::Default` with no implementation and
panics when `Exec::execute` is called.

Since `Exec::Default` has no purpose, it is being removed and user
should now provide an implementation of executor.

Closes hyperium#3128

BREAKING CHANGE:  `hyper::client::conn::Http2::Builder::new` now requires an executor argument.

Signed-off-by: Sven Pfennig <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
B-breaking-change Blocked: this is an "API breaking change". C-bug Category: bug. Something is wrong. This is bad!
Projects
No open projects
hyper 1.0
Status: Done
Milestone
1.0 RC3
Development

Successfully merging a pull request may close this issue.

feat(client): remove common::Exec::Default dswij/hyper
2 participants
@seanmonstar @conradludgate