Conversation
cogboard-app/src/main/kotlin/com/cognifide/cogboard/ssh/auth/SSHAuthData.kt
Outdated
Show resolved
Hide resolved
cogboard-app/src/main/kotlin/com/cognifide/cogboard/ssh/auth/SSHAuthData.kt
Outdated
Show resolved
Hide resolved
cogboard-app/src/main/kotlin/com/cognifide/cogboard/ssh/auth/SSHAuthData.kt
Show resolved
Hide resolved
| } | ||
|
|
||
| fun createCommand(): String { | ||
| val logLines = config.getString(CogboardConstants.Props.LOG_LINES) ?: "0" |
There was a problem hiding this comment.
Please use the overloaded getString method that uses the second parameter:
config.getString(CogboardConstants.Props.LOG_LINES, "0")
| connect(config) | ||
| } catch (e: JSchException) { | ||
| LOGGER.error(e.message) | ||
| vertx.eventBus().send(eventBusAddress, e) |
There was a problem hiding this comment.
Can we extract method here:
sendError(e)
|
|
||
| private fun initSSHSession(authData: SSHAuthData) { | ||
| jsch = JSch() | ||
| jsch.setKnownHosts("~/.ssh/known_hosts") |
There was a problem hiding this comment.
what does this line mean? - does it requires some configuration?
cogboard-app/src/main/kotlin/com/cognifide/cogboard/ssh/session/SessionStrategyFactory.kt
Show resolved
Hide resolved
|
|
||
| class SSHKeyAuthSessionStrategy(jSch: JSch, authData: SSHAuthData) : SessionStrategy(jSch, authData) { | ||
| override fun initSession(): Session { | ||
| if (authData.password == "") { |
There was a problem hiding this comment.
Please use: import io.netty.util.internal.StringUtil.EMPTY_STRING
Fix/review 414
Added proper exception handling to react to errors #420
| channel.inputStream = null | ||
| sshInputStream = channel.inputStream |
There was a problem hiding this comment.
I don't get this part: why do we set those nulls here?
| BASIC -> config.getString(Props.PASSWORD) | ||
| SSH_KEY -> config.getString(Props.SSH_KEY) |
There was a problem hiding this comment.
You have created variables for this: lines 12, 14
| import io.vertx.core.json.JsonArray | ||
| import io.vertx.core.json.JsonObject | ||
|
|
||
| class SSHAuthData(private val config: JsonObject) { |
There was a problem hiding this comment.
either store config as a variable or use only defined variables from config: lines 11-17
cogboard-app/src/main/kotlin/com/cognifide/cogboard/ssh/session/SessionStrategyFactory.kt
Show resolved
Hide resolved
| var message = "" | ||
| when { | ||
| !::vertx.isInitialized -> message = "Vertx instance not passed to builder" | ||
| !::eventBusAddress.isInitialized -> message = "Eventbus address not passed to builder" |
There was a problem hiding this comment.
message += - first message might be overriten
| assert(template.getString(0) == PROVIDER) | ||
| assert(template.getString(1) == MESSAGE) |
There was a problem hiding this comment.
I would rather see an actual expected strings here for a better understanding
# Conflicts: # cogboard-webapp/src/components/widgets/types/LogViewerWidget/helpers.js
LogsViewer - change logs generator permissions
#476 - LogsViewer - Changes on frontend
7 participants
DO NOT MERGE
This is just a changes preview fro CR purposes
SP: 1