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

[WIP] Refactor app management and routing logic. #310

Closed
wants to merge 46 commits into from

Conversation

evanlinjin
Copy link

@evanlinjin evanlinjin commented Apr 26, 2019

Outdates #281, Related to #278

Intentions

Code is confusing and hard to work on top of to implement new features (specifically in relation to #278).

The code in /pkg/app and related logic are currently implemented badly. In summary:

  • Naming of variables, members and structures contain too much repetition, and some names no not make any sense.
  • Some sections of logic is split between multiple modules and files, leading to reduced legibility.
  • Tests are uncommented and logic is ambiguous.

Changes

This is to be filled out as the PR progresses.

Within /pkg/app

  • Extracted net.Conn implementation via unix pipes (and associated structures) to an internal module /internal/appnet.
  • Renamed various structures in /pkg/app (and the newly created /internal/appnet) to make more logical sense.
  • (*appnet.Protocol).Serve() now uses a map as input (instead of a function) to make the distinction as to what actions are to be performed for what FrameType.
  • Rewrote tests in /pkg/app to make more sense.

Within /pkg/node

  • Removed appManager structure and moved the logic within a member of node.Node as there is no need to abstract this logic out).

林志宇 added 15 commits April 10, 2019 16:25
- Split app module into /internal/appnet and /pkg/app.
- Renamed app-management components for better readability.
- Simplified app-init process: now using sub-commands from app instead of InitFrame.
- Added another pair of unix pipes between app and it's host.
- encoding/gob is now used instead of json to improve performance.
- refactored 'appnet.Protocol' to have more consistent naming.
- updated tests for '/pkg/app'.
- Updated tests in '/internal/appnet', '/pkg/app' and '/pkg/router' modules.
- Fixed bug where (appnet.Protocol).Call() hangs when the associated appnet.HandlerFunc responds with error.
- Merge branch 'mainnet' into 'feature/manager-app-endpoints'.
- Updated tests to accommodate changes from merging.
- Fixed race condition in /pkg/apps tests.
- Removed 'version' field in node.AppConfig (version is now obtained from App's exec).
- Updated some tests.
- Introduced router.AppManager which replaces: router.loopList, router.portList, router.portManager, node.osExecutor
- Merged router.routeManger into router.Router
- Renamed router.managedRoutingTable to router.routingTableManager and introduced some logic originally from router.routeManager and router.Router
- Simplified router.Router and node.Node
- Added Start/Stop/Call logic to app.Host (originally spread across node.osExecutor and node.Node)
- Commented code and made 'make lint' pass.
- App now receives host's public key via env.
- Added 'router.AppInfo' structure to provide more complete app info.
- Improved sync.RWMutex management in 'router.AppsManager'
- Simplified '(*router.Router).Setup()' loop.
- Temporarily commented out tests that no longer pass due to changes.
- Renamed 'router.AppsManager' to 'router.ProcManager'.
- Introduced the 'ProcID' concept to 'router.ProcManager'.
- Renamed 'app.Host' to 'app.Executor'.
- Separated 'app.NewExecutor()' into 'app.ObtainMeta()' and 'app.NewExecutor()'.
- Temporary: commented out parts of the codebase that needs updating due to changes.
@evanlinjin evanlinjin closed this Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants