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.

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

feat: refactor ConnectionManager for maintainability #1969

Open
1 task
danisharora099 opened this issue Apr 22, 2024 · 1 comment
Open
1 task

feat: refactor ConnectionManager for maintainability #1969

danisharora099 opened this issue Apr 22, 2024 · 1 comment
Assignees
Labels
debt Technical debt marker enhancement New feature or request

Comments

@danisharora099
Copy link
Collaborator

danisharora099 commented Apr 22, 2024

This is a change request

Problem

We introduced a ConnectionManager a while ago to gain more control over how dials occur within a js-waku node, and slowly a lot of new APIs, refactors, etc started to bloat which is our current implementation.
Our current implementation is ~580 lines, and very coupled together -- making it quite unreadable and unmaintainable.

Proposed Solutions

Split the ConnectionManager class into multiple classes, aiming to adhere to the Single Responsibility Principle.

Suggestions for the split based on the current type defs for the class:

  • ConnectionManager: Responsible for overall connection management, including creating instances, starting and stopping the manager, and toggling online/offline status.
  • PeerDialer: Handles dial of peers, including attempting dials, managing dial attempts, and processing the dial queue.
  • PeerHandler: can be one module or can be split into two following modules:
    • PeerDiscoveryManager: Manages peer discovery events, including starting the discovery listener and dispatching discovery events.
    • PeerConnectionManager: Handles peer connection and disconnection events, including starting the connection and disconnection listeners, and managing connections.
  • PeerInfoManager: Provides utility functions for retrieving peer information, such as tag names, configured topics, and shard info.
    • Since this is more like a utility class, this can be avoided and the functions can stay part of the original ConnectionManager class being private
  • KeepAliveManager: Manages keep-alive functionality for connected peers. (exists already)

Subtasks

Notes

some inspiration here: https://github.com/libp2p/js-libp2p/tree/main/packages/libp2p/src/connection-manager

@weboko weboko added the enhancement New feature or request label Apr 24, 2024
@weboko
Copy link
Collaborator

weboko commented May 2, 2024

Postponed till we have #1595 done

@weboko weboko self-assigned this Oct 4, 2024
@weboko weboko changed the title chore: refactor ConnectionManager for maintainability feat: refactor ConnectionManager for maintainability Oct 8, 2024
@weboko weboko added the debt Technical debt marker label Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Technical debt marker enhancement New feature or request
Projects
Status: Priority
Development

No branches or pull requests

2 participants