GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-113538: Allow client connections to be closed by CendioOssman · Pull Request #116784 · python/cpython

Conversation

@CendioOssman

@CendioOssman CendioOssman commented Mar 14, 2024 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This is a new attempt at #114432, with one additional commit to fix unreliably tests.

  • Issue: Cannot cleanly shut down an asyncio based server #113538

📚 Documentation preview 📚: https://cpython-previews--116784.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
CendioOssman added 15 commits January 22, 2024 11:30
@CendioOssman
Don't use internals for wait_closed() tests
9f0111e
@CendioOssman
pythongh-113538: Allow client connections to be closed
c25eabb 
Give applications the option of more forcefully terminating client
connections for asyncio servers. Useful when terminating a service and
there is limited time to wait for clients to finish up their work.
@CendioOssman
Fix method references in new documentation
b7fa198
@CendioOssman
Add attribution
b3cd9c1
@CendioOssman
Only keep a weak client references
c78a927 
We want to be able to detect if the application fails to keep track of
the transports, so we cannot keep them alive by using a hard reference.
@CendioOssman
Inform server of fallback transport cleanup
1ec06da 
The application might be waiting for all transports to close, so we need
to properly inform the server that this transport is done.
@CendioOssman
Clarify recomended order of methods
2790ddf
@CendioOssman
Fix typos
6a56a80
@CendioOssman
Use discard() instead of conditional remove()
6929888
@CendioOssman
Improve some comments
8316199 
One could be made clearar, and the other is probably superfluous.
@CendioOssman
Do multiple sleeps to flush out callbacks
3e1705b
@CendioOssman
More deterministic abort_clients() test
6c078d6 
Try to get the streams and the kernel in to a more deterministic state
by specifying fixed buffering limits.
@CendioOssman
Even more deterministic
1158151 
No possibly infinite loop. Instead ask the system how much buffer space
it has and fill that.
@CendioOssman
Merge branch 'main' of https://github.com/python/cpython into server_…
1065dda 
…close
@CendioOssman
Wait for test client to fully connect
6a524cb 
In case it takes some time, as we need access to the server side
stream objects to perform the test.
@CendioOssman
CendioOssman requested review from 1st1, asvetlov, gvanrossum, kumaraditya303 and willingc as code owners March 14, 2024 08:43
@bedevere-app bedevere-app Bot added the awaiting review label Mar 14, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Mar 14, 2024
Cannot cleanly shut down an asyncio based server #113538
Closed
@CendioOssman

CendioOssman commented Mar 14, 2024

Copy link
Copy Markdown
Contributor Author

@gvanrossum, this should hopefully be more stable.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gvanrossum
gvanrossum approved these changes Mar 18, 2024
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

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.

LGTM, this looks like the right solution.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Mar 18, 2024
@gvanrossum
gvanrossum merged commit 4159644 into python:main Mar 18, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Mar 18, 2024
vstinner pushed a commit to vstinner/cpython that referenced this pull request Mar 20, 2024
@CendioOssman @vstinner
pythongh-113538: Add asycio.Server.{close,abort}_clients (redo) (pyth…
cc24c18 
…on#116784)

These give applications the option of more forcefully terminating client
connections for asyncio servers. Useful when terminating a service and
there is limited time to wait for clients to finish up their work.

This is a do-over with a test fix for pythongh-114432, which was reverted.
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@CendioOssman @adorilson
pythongh-113538: Add asycio.Server.{close,abort}_clients (redo) (pyth…
ac9d421 
…on#116784)

These give applications the option of more forcefully terminating client
connections for asyncio servers. Useful when terminating a service and
there is limited time to wait for clients to finish up their work.

This is a do-over with a test fix for pythongh-114432, which was reverted.
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@CendioOssman @diegorusso
pythongh-113538: Add asycio.Server.{close,abort}_clients (redo) (pyth…
01d051f 
…on#116784)

These give applications the option of more forcefully terminating client
connections for asyncio servers. Useful when terminating a service and
there is limited time to wait for clients to finish up their work.

This is a do-over with a test fix for pythongh-114432, which was reverted.
@encukou encukou mentioned this pull request Aug 28, 2024
gh-122136: test_asyncio: Don't fail if the kernel buffers more data than advertised #123423
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner
@willingc willingc Awaiting requested review from willingc willingc is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@CendioOssman @gvanrossum
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.