Implements addAdbPublicKey API endpoint #770
Conversation
Reason: manually approve adb connection from web UI does not scale for test automation. Especially those that are using different adb keys on each session (eg: via container) This changes also added swagger doc endpoint. Using `stf local` command, it will end up at: `http://localhost:7106/docs/`
lib/units/api/controllers/user.js
Outdated
| return { | ||
| key: { | ||
| title: data.title || key.comment | ||
| , fingerprint: key.fingerprint |
There was a problem hiding this comment.
Small thing but could you reduce the indentation on every line that starts with a comma? The fields should line up.
|
Is this feature will be merged soon ? |
| }) | ||
| }).catch(function(err) { | ||
| log.error('Failed to insert new adb key fingerprint: ', err.stack) | ||
| return res.status(500).json({ |
There was a problem hiding this comment.
Shouldn't we distinguish cases when supplied key is malformed and respond with error 400 in such case?
There was a problem hiding this comment.
adbkit does not provide a specific exception object we can catch. This is what adbkit do when encountering such input:
reject new Error "Unrecognizable public key format"
What we can do better here probably is passing through the error message. Would that be suffice?
There was a problem hiding this comment.
@koral-- , is it possible now to merge soon this interesting feature ?
There was a problem hiding this comment.
Sorry for the delay.
LGTM
What we can do better here probably is passing through the error message. Would that be suffice?
Stacktrace from that error is printed to the log so I think it is OK to leave it as is.
|
Actually, ADB keys are created through the UI, so for consistency is this feature includes the updating of the UI while creating ADB keys through the API ? |
|
It is a great API enhancement, do you consider adding a new API to delete the ADBKey or purge all adbKeys? |
@denis99999 |
@stevelibuzz |
| }) | ||
| }).catch(function(err) { | ||
| log.error('Failed to insert new adb key fingerprint: ', err.stack) | ||
| return res.status(500).json({ |
There was a problem hiding this comment.
Sorry for the delay.
LGTM
What we can do better here probably is passing through the error message. Would that be suffice?
Stacktrace from that error is printed to the log so I think it is OK to leave it as is.
|
Finally merged! Thanks :) |
Reason: manually approve adb connection from web UI does not scale for test automation. Especially those that are using different adb keys on each session (eg: via container)
This changes also added swagger doc endpoint. Using
stf localcommand, it will end up at:http://localhost:7106/docs/