Add ssh-plugin #31
Add ssh-plugin #31
Conversation
|
FYI seems there is a conflict |
| @@ -0,0 +1,47 @@ | |||
| /********************************************************************* | |||
| * Copyright (c) 2018 Red Hat, Inc. | |||
| try { | ||
| const client = await this.wsClient(); | ||
| if (client) { | ||
| return await client.generateSshKey(service, name); |
There was a problem hiding this comment.
it looks odd to return await as we're returning a promise ?
| return await client.generateSshKey(service, name); | ||
| } | ||
|
|
||
| return Promise.reject(`Unable to generate SSH Key for ${service}:${name}`); |
There was a problem hiding this comment.
with async methods we can just do throw new Error()
| try { | ||
| const client = await this.wsClient(); | ||
| if (client) { | ||
| return await client.createSshKey(sshKeyPair); |
There was a problem hiding this comment.
why do we need a return and await ?
| try { | ||
| const client = await this.wsClient(); | ||
| if (client) { | ||
| return await client.getAllSshKey(service); |
| * @inheritDoc | ||
| */ | ||
| create(sshKeyPair: cheApi.ssh.SshPair): Promise<void> { | ||
| return new Promise<void>((resolve, reject) => { |
There was a problem hiding this comment.
here why are we not using async/await as well ?
|
|
||
| import * as theia from '@theia/plugin'; | ||
| import { RemoteSshKeyManager, SshKeyManager, CheService } from './node/ssh-key-manager'; | ||
| import WorkspaceClient from '@eclipse-che/workspace-client'; |
There was a problem hiding this comment.
why do we need workspace-client ?
AFAIK @eclipse-che/plugin should provide the access to ssh no ?
| @@ -0,0 +1,43 @@ | |||
| /********************************************************************* | |||
| * Copyright (c) 2018 Red Hat, Inc. | |||
|
|
||
| throw new Error(`Unable to generate SSH Key for ${service}:${name}`); | ||
| } catch (e) { | ||
| throw new Error('Unable to create Che API REST Client'); |
There was a problem hiding this comment.
should we rethrow original error ? here message is lost.
There was a problem hiding this comment.
yep, you are right
|
|
||
| throw new Error(`Unable to create SSH Key`); | ||
| } catch (e) { | ||
| throw new Error('Unable to create Che API REST Client'); |
There was a problem hiding this comment.
if try () catch is only for
await this.wsClient()
why not throw an error instead of returning undefined ?
also try / catch block should be just around this.wsClient() if kept like this
note: this comment applies to all get***() methods
| @@ -0,0 +1,78 @@ | |||
| /********************************************************************* | |||
| * Copyright (c) 2018 Red Hat, Inc. | |||
| @@ -22,6 +22,9 @@ module.exports = { | |||
| use: [ | |||
| { | |||
| loader: 'ts-loader', | |||
| options: { | |||
| transpileOnly: true | |||
There was a problem hiding this comment.
hi, why do we need this change ?
There was a problem hiding this comment.
this is temporary, in some reason got error form webpack
| export namespace ssh { | ||
| export function generate(service: string, name: string): Promise<cheApi.ssh.SshPair>; | ||
|
|
||
| export function create(sshKeyPair: cheApi.ssh.SshPair): Promise<void>; |
There was a problem hiding this comment.
should it be PromiseLike in return type ?
Signed-off-by: Vitalii Parfonov <vparfonov@redhat.com>
plugins/ssh-plugin/tsconfig.json
Outdated
| "include": [ | ||
| "src" | ||
| ] | ||
| } |
plugins/ssh-plugin/package.json
Outdated
| "@theia/plugin-packager": "latest", | ||
| "rimraf": "2.6.2", | ||
| "typescript-formatter": "7.2.2", | ||
| "typescript": "2.9.2", |
There was a problem hiding this comment.
please remove all devDependencies that are already in the parent folder (yarn workspace)
There was a problem hiding this comment.
also probably axios is no longer used there ?
Signed-off-by: Vitalii Parfonov <vparfonov@redhat.com>
Signed-off-by: Oleksandr Garagatyi <ogaragat@redhat.com>
eclipse-che/che#12306
Signed-off-by: Vitalii Parfonov vparfonov@redhat.com