Update Python demo servers to use async boilerplate #645

pepone · 2025-10-16T14:05:34Z

No description provided.

Copilot

Pull Request Overview

Modernizes Python demo servers to use asyncio-based boilerplate for Ice communicators, improving graceful shutdown and consistency across demos.

Convert main entry points to async and run with asyncio.run
Integrate Ice with asyncio event loops and replace waitForShutdown with await shutdownCompleted
Add SIGINT handlers to trigger communicator shutdown

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
python/IceGrid/greeter/server/main.py	Switch to asyncio integration and await shutdown completion.
python/IceDiscovery/replication/server/main.py	Configure Ice with asyncio EventLoopAdapter; add SIGINT handler and await shutdown.
python/IceDiscovery/greeter/server/main.py	Configure Ice with asyncio EventLoopAdapter; add SIGINT handler and await shutdown.
python/Ice/optional/server2/main.py	Adopt asyncio loop and graceful shutdown pattern.
python/Ice/optional/server1/main.py	Adopt asyncio loop and graceful shutdown pattern.
python/Ice/greeter/server_asyncio/main.py	Pass sys.argv into async initialize; keep asyncio-based pattern.
python/Ice/customError/server/main.py	Adopt asyncio loop and graceful shutdown pattern.
python/Ice/context/server/main.py	Adopt asyncio loop and graceful shutdown pattern.
python/Ice/config/server/main.py	Build InitializationData with asyncio EventLoopAdapter; await shutdown.

Comments suppressed due to low confidence (2)

Copilot · 2025-10-16T14:09:05Z

python/IceDiscovery/replication/server/main.py

 import uuid

 import chatbot
 import Ice


Ice.asyncio.EventLoopAdapter is used but the submodule is not imported. Without importing Ice.asyncio, this will raise AttributeError at runtime. Add import Ice.asyncio alongside the other imports.

Suggested change

import Ice

import Ice

import Ice.asyncio

Copilot · 2025-10-16T14:09:05Z

python/IceDiscovery/greeter/server/main.py

 import sys

 import chatbot
 import Ice


Ice.asyncio.EventLoopAdapter is referenced but the submodule is not imported. Add import Ice.asyncio to ensure EventLoopAdapter is available.

Suggested change

import Ice

import Ice

import Ice.asyncio

Copilot · 2025-10-16T14:09:05Z

python/Ice/config/server/main.py

 import sys

 import chatbot
 import Ice


Ice.asyncio.EventLoopAdapter is used below, but Ice.asyncio is not imported. Add import Ice.asyncio so the adapter is resolvable at runtime.

Suggested change

import Ice

import Ice

import Ice.asyncio

Copilot · 2025-10-16T14:09:06Z

python/IceGrid/greeter/server/main.py

+        # Shutdown the communicator when the user presses Ctrl+C.
+        loop.add_signal_handler(signal.SIGINT, communicator.shutdown)


[nitpick] Also handle SIGTERM for graceful shutdowns in environments that send terminate signals (e.g., systemd, Kubernetes). Consider adding loop.add_signal_handler(signal.SIGTERM, communicator.shutdown). On Windows, add_signal_handler may raise NotImplementedError; wrapping these calls in a try/except NotImplementedError keeps the demo cross-platform.

Suggested change

# Shutdown the communicator when the user presses Ctrl+C.

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

# Shutdown the communicator when the user presses Ctrl+C or receives SIGTERM.

try:

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

loop.add_signal_handler(signal.SIGTERM, communicator.shutdown)

except NotImplementedError:

# Signal handlers are not supported on Windows

pass

Copilot · 2025-10-16T14:09:06Z

python/Ice/optional/server2/main.py

+        # Shutdown the communicator when the user presses Ctrl+C.
+        loop.add_signal_handler(signal.SIGINT, communicator.shutdown)


[nitpick] Add a SIGTERM handler for graceful termination: loop.add_signal_handler(signal.SIGTERM, communicator.shutdown). Consider wrapping add_signal_handler in try/except NotImplementedError for Windows.

Suggested change

# Shutdown the communicator when the user presses Ctrl+C.

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

# Shutdown the communicator when the user presses Ctrl+C or when receiving SIGTERM.

try:

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

loop.add_signal_handler(signal.SIGTERM, communicator.shutdown)

except NotImplementedError:

# Signal handlers are not implemented on Windows for asyncio event loops.

pass

Copilot · 2025-10-16T14:09:06Z

python/Ice/customError/server/main.py

+    # support by passing the current event loop to initialize.
+    with Ice.initialize(sys.argv, eventLoop=loop) as communicator:
+        # Shutdown the communicator when the user presses Ctrl+C.
+        loop.add_signal_handler(signal.SIGINT, communicator.shutdown)


[nitpick] Handle SIGTERM as well to support graceful shutdown in production-like environments: loop.add_signal_handler(signal.SIGTERM, communicator.shutdown). Optionally guard with try/except NotImplementedError for Windows.

Suggested change

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

# Also handle SIGTERM for graceful shutdown in production environments.

try:

loop.add_signal_handler(signal.SIGTERM, communicator.shutdown)

except NotImplementedError:

# SIGTERM handling is not supported on Windows

pass

Copilot · 2025-10-16T14:09:07Z

python/IceDiscovery/replication/server/main.py

    with Ice.initialize(initData=initData) as communicator:
+        # Shutdown the communicator when the user presses Ctrl+C.
+        loop.add_signal_handler(signal.SIGINT, communicator.shutdown)
+


[nitpick] Add SIGTERM handling to support common service lifecycles: loop.add_signal_handler(signal.SIGTERM, communicator.shutdown). Optionally wrap in try/except NotImplementedError for Windows compatibility.

Suggested change

# Also handle SIGTERM for graceful shutdown in service environments.

try:

loop.add_signal_handler(signal.SIGTERM, communicator.shutdown)

except NotImplementedError:

# SIGTERM handling is not supported on Windows

pass

Copilot · 2025-10-16T14:09:07Z

python/IceDiscovery/greeter/server/main.py

    # Create an Ice communicator. We'll use this communicator to create an object adapter.
    with Ice.initialize(initData=initData) as communicator:
+        # Shutdown the communicator when the user presses Ctrl+C.
+        loop.add_signal_handler(signal.SIGINT, communicator.shutdown)


[nitpick] Recommend adding loop.add_signal_handler(signal.SIGTERM, communicator.shutdown) to gracefully handle termination signals (e.g., from orchestrators). Consider a try/except NotImplementedError guard for Windows.

Suggested change

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

# Also handle SIGTERM for graceful shutdown (e.g., from orchestrators).

try:

loop.add_signal_handler(signal.SIGTERM, communicator.shutdown)

except NotImplementedError:

# SIGTERM handling is not implemented on Windows

pass

Copilot · 2025-10-16T14:09:08Z

python/Ice/greeter/server_asyncio/main.py

        # Shutdown the communicator when the user presses Ctrl+C.
        loop.add_signal_handler(signal.SIGINT, communicator.shutdown)


[nitpick] Add SIGTERM handling as well for better shutdown behavior in managed environments: loop.add_signal_handler(signal.SIGTERM, communicator.shutdown). You may also guard add_signal_handler with try/except NotImplementedError for Windows.

Suggested change

# Shutdown the communicator when the user presses Ctrl+C.

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

# Shutdown the communicator when the user presses Ctrl+C or receives SIGTERM.

try:

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

loop.add_signal_handler(signal.SIGTERM, communicator.shutdown)

except NotImplementedError:

# Signal handlers are not supported on this platform (e.g., Windows)

pass

Copilot · 2025-10-16T14:09:08Z

python/Ice/config/server/main.py

+        # Shutdown the communicator when the user presses Ctrl+C.
+        loop.add_signal_handler(signal.SIGINT, communicator.shutdown)


[nitpick] Add SIGTERM handling to complete graceful shutdown coverage: loop.add_signal_handler(signal.SIGTERM, communicator.shutdown). Consider wrapping add_signal_handler in try/except NotImplementedError for Windows platforms.

Suggested change

# Shutdown the communicator when the user presses Ctrl+C.

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

# Shutdown the communicator when the user presses Ctrl+C or receives SIGTERM.

loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

try:

loop.add_signal_handler(signal.SIGTERM, communicator.shutdown)

except NotImplementedError:

# SIGTERM handling is not implemented on Windows

pass

externl

What about Ice/greeter/server?

pepone · 2025-10-16T15:04:01Z

Ice/greeter/server?

I didn't updated it because we have server_asyncio in the same demo, but maybe we should too.

bernardnormier

I would not catch SIGTERM as suggested by Copilot; just keep it simple.

bernardnormier · 2025-10-16T15:03:46Z

python/Ice/config/server/main.py

+    initData = Ice.InitializationData()
+
    # Create a Properties object from the command line arguments and the config file properties; Ice.* properties and
    # other reserved properties set in sys.argv augment or override the config file properties.


set in the sys.argv command-line arguments (not augment)

bernardnormier · 2025-10-16T15:06:56Z

python/Ice/greeter/server_asyncio/main.py

    # Create an Ice communicator. We'll use this communicator to create an object adapter. We enable asyncio
    # support by passing the current event loop to initialize.
-    async with Ice.initialize(eventLoop=loop) as communicator:
+    async with Ice.initialize(sys.argv, eventLoop=loop) as communicator:


We should rename this server in a follow-up PR. Since everything uses asyncio, we should not name a server "server_asyncio".

Update Python demo servers to use async boilerplate

f69bf08

pepone requested review from bernardnormier, Copilot and externl October 16, 2025 14:05

Copilot AI reviewed Oct 16, 2025

View reviewed changes

externl reviewed Oct 16, 2025

View reviewed changes

bernardnormier approved these changes Oct 16, 2025

View reviewed changes

Review fixes

443bdd6

pepone merged commit 4a7267c into zeroc-ice:main Oct 17, 2025
16 checks passed

		# Shutdown the communicator when the user presses Ctrl+C.
		loop.add_signal_handler(signal.SIGINT, communicator.shutdown)

-        # Shutdown the communicator when the user presses Ctrl+C.
-        loop.add_signal_handler(signal.SIGINT, communicator.shutdown)
+        # Shutdown the communicator when the user presses Ctrl+C or receives SIGTERM.
+        try:
+            loop.add_signal_handler(signal.SIGINT, communicator.shutdown)
+            loop.add_signal_handler(signal.SIGTERM, communicator.shutdown)
+        except NotImplementedError:
+            # Signal handlers are not supported on Windows
+            pass

Update Python demo servers to use async boilerplate #645

Update Python demo servers to use async boilerplate #645

Uh oh!

Conversation

pepone commented Oct 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

externl left a comment

Choose a reason for hiding this comment

Uh oh!

pepone commented Oct 16, 2025

Uh oh!

bernardnormier left a comment

Choose a reason for hiding this comment

Uh oh!

bernardnormier Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

bernardnormier Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bernardnormier Oct 16, 2025 •

edited

Loading