refactor: adapt wrong imports in tck and sample#948
Conversation
🧪 Code Coverage (vs
|
🧪 Code Coverage (vs
|
There was a problem hiding this comment.
Code Review
This pull request introduces significant updates to the A2A SDK, including a transition to Protobuf-based serialization, the addition of a database migration CLI, and support for legacy v0.3 protocol compatibility. The changes include new migration scripts, updated transport implementations, and improved request handling. I have reviewed the changes and agree with the suggestion to replace the abrupt os._exit(0) call in the CLI sample with a graceful KeyboardInterrupt handler to ensure proper resource cleanup.
I am having trouble creating individual review comments. Click here to see my feedback.
samples/cli.py (124-125)
The use of os._exit(0) in the signal handler provides an abrupt termination of the program. This will prevent any cleanup code, such as the await client.close() call in your main function, from executing. This can lead to resource leaks, like open network connections.
A more graceful approach is to let asyncio handle SIGINT by raising a KeyboardInterrupt, which can then be caught to perform a clean shutdown.
I suggest removing the signal handler and wrapping the asyncio.run() call in a try...except KeyboardInterrupt block. This will ensure that client.close() is called when the user exits with Ctrl+C.
try:
asyncio.run(main())
except KeyboardInterrupt:
print('\nExiting gracefully...')
Description
This PR fixes the wrong imports after the introduction of new default_request_handler_V2