-
Notifications
You must be signed in to change notification settings - Fork 189
Replace async service with aniyo service #973
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
base: main
Are you sure you want to change the base?
Conversation
|
@parth-soni07 : Thank you so much for sharing this PR. Appreciate your great efforts and contribution. We will jointly do a detailed review. Will share feedback soon. Wish if you could resolve the merge conflicts. Once resolved, please ping me. I will re-run the CI/CD pipeline and share test results soon. Also, CCing @acul71, @sumanjeet0012 and @pacrob. |
|
@parth-soni07 I have reviewed the PR and have a few observations: |
Hey @sumanjeet0012 , thank you for your insights, yes indeed this is supposed to be a anyio migration and not trio, I have made some changes in the implementation to align with the name of the PR and use anyio. I am facing some issues in the version of anyio that is used in the p2pclient so currently working on that, will definitely ping once I am done with that. |
|
Hello @parth-soni07 thanks for this PR. AI Pull Request Review: PR #973PR Title: Replace async service with aniyo service 1. Summary of ChangesThis PR implements a major refactoring that replaces the legacy Understanding the Service Framework EcosystemTo understand the significance of this migration, it's important to understand the role of service frameworks in py-libp2p and the broader async Python ecosystem: async_service (Legacy - Being Replaced)The original
However, the upstream library became unmaintained, leaving py-libp2p with a frozen codebase that lacked modern async patterns, bug fixes, and improvements. The code was tightly coupled to Trio's async primitives (nurseries, cancellation scopes) and didn't leverage newer structured concurrency patterns. Trio (The Async Runtime)Trio is a modern, production-grade async framework for Python that emphasizes:
py-libp2p has been built on Trio from the beginning because its structured concurrency model aligns perfectly with libp2p's needs:
All core py-libp2p services (Swarm, PubSub, KadDHT, Circuit Relay) inherit from the AnyIO (The Compatibility Layer)AnyIO is a compatibility library that provides a unified async API that works with multiple backends:
AnyIO's key benefits:
anyio_service (The New Implementation)The new
The Relationship in py-libp2pIn py-libp2p's architecture: Current State (After this PR):
Why This Matters:
What Changed:
Breaking Changes:
2. Strengths
3. Issues FoundCritical
Major
Minor
4. Security ReviewSecurity Impact: None / Low No security vulnerabilities identified. The changes are primarily architectural refactoring:
Potential Considerations:
5. Documentation and ExamplesStrengths:
Issues:
6. Newsfragment RequirementRequired Action:File: Content should be: Replaced legacy async_service implementation with modern anyio_service framework built on AnyIO for better structured concurrency, cross-platform compatibility, and improved maintainability.Requirements:
Type Selection:
Impact: PR cannot be approved without this newsfragment file. This is a mandatory requirement, not optional. 7. Tests and ValidationTest Coverage:New Tests Added:
Test Updates:
Issues:
Build and Linting:Status: ❌ FAILED - Both Results Summary:✅ Passed:
❌ Failed:
Type Checking Errors Found:The type checkers (mypy and pyrefly) are reporting multiple issues with the AnyIO integration:
Root Cause Analysis:
Recommendation:
Action Required: These type checking errors must be resolved before merge, as they may indicate actual runtime issues or will cause CI to fail. Runtime Import Errors (CRITICAL):Status: ❌ FAILED - Error: Impact:
Root Cause: from anyio.abc import ObjectReceiveStream, ObjectSendStreamThese classes either:
Files Affected:
Action Required:
8. Recommendations for Improvement
9. Questions for the Author
10. Overall AssessmentQuality Rating: Needs WorkThe PR represents a well-structured migration with comprehensive code updates, good modular design, and maintained backward compatibility. However:
The architectural design is sound, but the implementation has critical issues that must be resolved before merge. Security Impact: None / LowNo security concerns identified. The refactoring improves error handling and resource management. Merge Readiness: Needs fixesBlockers:
Before Merge:
After Merge Considerations:
Confidence: LowThe code changes are well-structured and comprehensive architecturally, but:
The import errors are a critical blocker - the code cannot be tested or used until these are fixed. This suggests the AnyIO integration may not have been fully tested or the API usage is incorrect for the AnyIO version being used. Additional Context from Discussion #1008The related discussion #1008 highlights a dependency conflict with
This context is important for understanding the dependency management strategy and ensuring the anyio version constraint works across all dependencies. Review completed: 2025-11-13
Next Steps: Author should address blockers, particularly the critical import errors (highest priority) and missing newsfragment, before requesting re-review. |
- Introduced hashlib for SHA-256 hashing in routing_table.py, utils.py, and transport.py. - Updated peer ID generation in id.py to use SHA-256 for larger keys and identity multihash for smaller keys. - Adjusted tests in test_peerid.py to reflect the new hashing approach. - Added missing import statements for hashlib where necessary.
Fix/replace async with anyio
What was wrong?
The codebase previously relied on a custom async_service implementation, which was no longer actively maintained and introduced unnecessary complexity. It made lifecycle management fragile and lacked support for modern async patterns and structured concurrency.
Issue #524
How was it fixed?
The legacy async_service implementation was fully replaced with a new design built using anyio, a well-supported, high-level async framework. This transition modernizes the service infrastructure and significantly improves reliability, maintainability, and task supervision.