core: terminalservice fork across cluster #1721
Conversation
plugins/core/src/terminal-service.ts
Outdated
| const fork = sdk.fork<{ | ||
| connectTTYStream: typeof connectTTYStream, | ||
| }>({ clusterWorkerId }); | ||
| const result = await fork.result; |
There was a problem hiding this comment.
this fork needs to be exited somehow
plugins/core/src/terminal-service.ts
Outdated
| connectTTYStream: typeof connectTTYStream, | ||
| }>({ clusterWorkerId }); | ||
| const result = await fork.result; | ||
| return result.connectTTYStream(input, options); |
There was a problem hiding this comment.
if fork.result throws, or connectTTYstream throws, I would also call fork.terminate to force immediate cleanup.
plugins/core/src/terminal-service.ts
Outdated
| } | ||
|
|
||
| export async function newTerminalService(): Promise<TerminalService> { | ||
| return new TerminalService(null, true); |
There was a problem hiding this comment.
this may cause weird issues creating another scrtyped device with a native id not belonging to it (undefined/null is the plugin). I'd use the correct native id
There was a problem hiding this comment.
I'm not sure how important this parameter is given the primary device is constructed like this: https://github.com/koush/scrypted/blob/main/plugins/core/src/main.ts#L245
It's probably more of an issue if it needs access to storage, which we don't use in the class today
There was a problem hiding this comment.
If you'd like I can also fix the primary device
| @@ -242,7 +242,7 @@ class ScryptedCore extends ScryptedDeviceBase implements HttpRequestHandler, Dev | |||
| if (nativeId === UsersNativeId) | |||
| return this.users ||= new UsersCore(); | |||
| if (nativeId === TerminalServiceNativeId) | |||
| return this.terminalService ||= new TerminalService(); | |||
| return this.terminalService ||= new TerminalService(TerminalServiceNativeId, false); | |||
There was a problem hiding this comment.
oh weird what the hell was this a bug
Enable
terminalserviceto fork across cluster nodes. This allows TTY plugins to tellterminalservicewhich node to launch its PTY.