Skip to content

conform to quinn api changes#56

Merged
stammw merged 9 commits intohyperium:masterfrom
FrankSpitulski:master
Nov 24, 2021
Merged

conform to quinn api changes#56
stammw merged 9 commits intohyperium:masterfrom
FrankSpitulski:master

Conversation

@FrankSpitulski
Copy link
Contributor

quinn had some api changes, so I've updated h3 to work with the new api on the master branch. marked as WIP since there hasn't been an 0.8.0 release with the new api yet.

@stammw
Copy link
Contributor

stammw commented Nov 3, 2021

Nice! Looks like the transport init code is a lot simpler now! I'll test this soon.

Did you have a look in tests ? I think helpers might need some changes too ?

@FrankSpitulski
Copy link
Contributor Author

I updated the examples, but it seemed to compile fine as is (which was my goal to make it work with my downstream project).

Since you’ve got a major refactor coming in I think it can just wait to see which drops first, quinn 0.8.0 or the h3 refactor.

@FrankSpitulski
Copy link
Contributor Author

@stammw I rebased and updated the tests package. the "test connect" test is failing now with a generic error. "h3::Error { code: H3_STREAM_CREATION_ERROR, cause: ApplicationClosed(ApplicationClose { error_code: 0, reason: b"" }) }"

I tried working out the exact issue but I'm very unfamiliar with the manual async style polling code. On my real world application though it seems to work just fine with the update.

@FrankSpitulski FrankSpitulski marked this pull request as ready for review November 16, 2021 00:05
@stammw
Copy link
Contributor

stammw commented Nov 21, 2021

All right, I'll have a look at this.

@stammw
Copy link
Contributor

stammw commented Nov 21, 2021

Could you use cargo fmt --all to fix the style ?

Copy link
Contributor

@stammw stammw left a comment

Choose a reason for hiding this comment

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

Almost ready! Can you please make sure other server implementations still answer the client example ?

@stammw
Copy link
Contributor

stammw commented Nov 23, 2021

And also:

Now the CI doesn't pass because the new version of rustls uses combinators on booleans. Can you add a commit like this one g2p/rustls-native-certs@0e1a013 ?

@stammw
Copy link
Contributor

stammw commented Nov 24, 2021

Did you check that server and client examples still work together ?

@stammw
Copy link
Contributor

stammw commented Nov 24, 2021

The only missing thing is the client example against server example case. Then we can merge it.

It appears to be broken:

❯ cargo run --example=server selfsigned
❯ cargo run --example=client -- --insecure https://localhost:4433
Error: TransportError(Error { code: Code::crypto(78), frame: None, reason: "ALPN negotiation failed" })

Copy link
Contributor

@stammw stammw left a comment

Choose a reason for hiding this comment

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

Looks like Github actions takes this as a float and removes the trailing 0...

@stammw stammw merged commit 7618753 into hyperium:master Nov 24, 2021
@stammw
Copy link
Contributor

stammw commented Nov 24, 2021

Thanks!

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