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

feat(request-response): deprecate Config::set_connection_keep_alive #4029

Merged
merged 16 commits into from
Oct 18, 2023
Merged

feat(request-response): deprecate Config::set_connection_keep_alive #4029

merged 16 commits into from
Oct 18, 2023

Conversation

thomaseizinger
Copy link
Contributor

@thomaseizinger thomaseizinger commented Jun 5, 2023 •
edited
Loading

Description

We deprecate the set_connection_keep_alive function in preparation for removing the KeepAlive::Until entirely. This is backwards-compatible.

Notes & open questions

At the moment, having to add keep_alive::Behaviour to things makes this rather clunky. We could fix this with a proposal like #3844 (comment).

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@thomaseizinger thomaseizinger requested a review from mxinden June 5, 2023 14:32
@thomaseizinger thomaseizinger mentioned this pull request Jun 5, 2023
Merged
4 tasks
@mxinden
Copy link
Member

mxinden commented Jun 26, 2023

Direction here looks good to me.

@thomaseizinger thomaseizinger mentioned this pull request Jun 26, 2023
Closed
@thomaseizinger
Copy link
Contributor Author

Direction here looks good to me.

I want to ship #4121 first so that we can avoid the use of keep_alive::Behaviour for the perf protocol.

@thomaseizinger thomaseizinger added this to the Simplify idle connection management milestone Sep 17, 2023
@thomaseizinger thomaseizinger mentioned this pull request Sep 19, 2023
Closed
@thomaseizinger thomaseizinger removed this from the Simplify idle connection management milestone Sep 19, 2023
@thomaseizinger thomaseizinger marked this pull request as ready for review September 20, 2023 02:39
@thomaseizinger thomaseizinger changed the title feat(request-response): only keep connection alive while we are using it feat(request-response): deprecate Config::set_connection_keep_alive Sep 20, 2023
This was referenced Sep 24, 2023
Closed
Merged
@mergify

This comment was marked as resolved.

Sign in to view
@thomaseizinger thomaseizinger mentioned this pull request Sep 28, 2023
Closed
@mergify

This comment was marked as resolved.

Sign in to view
@thomaseizinger thomaseizinger mentioned this pull request Oct 13, 2023
Merged
4 tasks
@thomaseizinger
Copy link
Contributor Author

Friendly ping @mxinden.

@thomaseizinger thomaseizinger mentioned this pull request Oct 16, 2023
Merged
4 tasks
@mergify

This comment was marked as resolved.

Sign in to view
@thomaseizinger thomaseizinger mentioned this pull request Oct 17, 2023
Closed
4 tasks
@mxinden mxinden mentioned this pull request Oct 17, 2023
Merged
4 tasks
@thomaseizinger thomaseizinger added the internal-change Pull requests that make internal changes to crates and thus don't need to include a changelog entry. label Oct 17, 2023
@thomaseizinger
Copy link
Contributor Author

Marking as internal-change for the changes in perf/.

mxinden
mxinden approved these changes Oct 18, 2023
View reviewed changes
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks.

@thomaseizinger
Copy link
Contributor Author

@mergify refresh

@mergify
Copy link
Contributor

mergify bot commented Oct 18, 2023

refresh

✅ Pull request refreshed

@mergify mergify bot merged commit 407dd42 into master Oct 18, 2023
@mergify mergify bot deleted the feat/3844-request-response branch October 18, 2023 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mxinden mxinden mxinden approved these changes

Assignees
No one assigned
Labels
internal-change Pull requests that make internal changes to crates and thus don't need to include a changelog entry. send-it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@thomaseizinger @mxinden