feat(api): add porcelain api with connect#1779
Conversation
Greptile SummaryThis PR introduces a
Confidence Score: 4/5Safe to merge after fixing the silent-host-ignore bug in Dimos.connect(); remaining findings are P2 style. One P1 defect: Dimos.connect(host="x") silently ignores the host and connects to localhost instead, which could cause confusing misdirected connections in production. The rest are P2 style/quality concerns (private API access in LocalModuleSource, lock held during network I/O). dimos/porcelain/dimos.py (P1 logic bug in connect()), dimos/porcelain/local_module_source.py (P2 style) Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Dimos
participant LocalModuleSource
participant RemoteModuleSource
participant ModuleCoordinator
participant RpycServer
participant Worker as PythonWorker (subprocess)
Note over User,Worker: Local path (app.run)
User->>Dimos: run(target)
Dimos->>ModuleCoordinator: build(blueprint)
ModuleCoordinator->>Worker: deploy / start modules
ModuleCoordinator->>RpycServer: start() - coordinator port saved
Dimos->>LocalModuleSource: created with coordinator ref
User->>Dimos: app.skills.ping()
Dimos->>LocalModuleSource: get_rpyc_module("StressTestModule")
LocalModuleSource->>Worker: actor.start_rpyc() - worker port
LocalModuleSource->>Worker: rpyc.connect(localhost, port)
Worker-->>LocalModuleSource: module proxy
LocalModuleSource-->>Dimos: module proxy
Dimos-->>User: skill result
Note over User,Worker: Remote path (Dimos.connect)
User->>Dimos: connect(host, port)
Dimos->>RemoteModuleSource: connect to coordinator RPyC port
RemoteModuleSource->>RpycServer: list_modules() / get_module_endpoint(name)
RpycServer->>ModuleCoordinator: delegate
RemoteModuleSource->>Worker: rpyc.connect(host, worker_port)
Worker-->>RemoteModuleSource: module proxy
RemoteModuleSource-->>User: skill result
Reviews (1): Last reviewed commit: "feat(api): add porcelain api with connec..." | Re-trigger Greptile |
889de47 to
cb78739
Compare
leshy
left a comment
There was a problem hiding this comment.
big comment on actual protocol to talk to workers, but I'm aproving as we iterate. protocol replacable later also.
Another
run() and restart() are only available in local mode. On a connected instance they raise NotImplementedError.
IMO we especially want those in remote mode:
- you start dimos, then claude iterates on the code on individual module, it wants to restart that module
- openclaw controls dimos, wants to deploy a new module
I'm not totally sure why those features are not "basically for free" and require extra work
cb78739 to
77113a6
Compare
Implemented this. Required pickling classes. |
Problem
Closes DIM-730
Solution
See the docs for how it works.
Breaking Changes
How to Test
Included in the docs.
Contributor License Agreement