-
Notifications
You must be signed in to change notification settings - Fork 595
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
Expose connection resumption details #1899
Conversation
Benchmark resultsInstruction countsSignificant differencesThere are no significant instruction count differences Other differencesClick to expand
Wall-timeSignificant differencesThere are no significant wall-time differences Other differencesClick to expand
Additional informationCheckout details:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1899 +/- ##
=======================================
Coverage 95.48% 95.49%
=======================================
Files 86 86
Lines 18624 18647 +23
=======================================
+ Hits 17784 17807 +23
Misses 840 840 ☔ View full report in Codecov by Sentry. |
Is there any vocabulary in the spec that we could reuse? I do feel like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm bikeshedding the enum name and suggesting some doc changes but it looks great broadly.
This allows callers to see if their handshake was Resumed, Full, or Full-with-HelloRetryRequest (which, broadly, are the three "cost" levels for handshakes). This is exposed as soon as it is known for sure.
a3cdc04
to
bf9bb68
Compare
/// | ||
/// This will return `Err(Error::HandshakeNotComplete)` before it is | ||
/// known which sort of handshake occurred. | ||
pub fn handshake_kind(&self) -> Result<HandshakeKind, Error> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little surprised at the choice of Result
here? IIRC most of the similar methods just return Option
, which seems appropriate here, too?
My goal with this is to support implementation of
SSL_session_reused
but this should also fix #705.I'm hoping what and how this works is relatively uncontroversial, but I'm not 100% pleased with the naming.