Draft: PDP-DEVTOOL 6434: Dev Assist proxy:generatekey command#1010
Draft: PDP-DEVTOOL 6434: Dev Assist proxy:generatekey command#1010TheJorgeBorras wants to merge 13 commits intodevfrom
Conversation
| "UTILS_CLIENT_API_KEY_UTILS_ERRORS_INVALID_FILE_CONTENTS": "Client API contents must be a valid format. Please, delete client_api_key.p12 file and try again.", | ||
| "UTILS_CLIENT_API_KEY_UTILS_ERRORS_READING_FILE_CONTENTS": "Error reading Client API Key file contents.\n{0}", | ||
| "UTILS_CLIENT_API_KEY_UTILS_ERRORS_WRITING_FILE_CONTENTS": "Error writing Client API Key file contents.", | ||
| "UTILS_CLIENT_API_KEY_UTILS_READING_FILE_CONTENTS": "Reading Client API contents...", | ||
| "UTILS_CLIENT_API_KEY_UTILS_WRITING_FILE_CONTENTS": "Writing Client API contents...", |
There was a problem hiding this comment.
Verify with Montsant
| // TODO: we do not want to show the content of the file | ||
| // Therefore, we dont want to show the contents of the caughtError (due to the risk of it sharing unwanted info) |
There was a problem hiding this comment.
Remove comment before merging PR
| const existingFileContent = await readClientAPIKeyFileContents(this._sdkExecutor); | ||
| const clientApiKeyFileContents = ClientApiKeyDTO.Builder | ||
| .fromRawString(existingFileContent.data) | ||
| .withNewProxyKey(newApiKey) | ||
| .build(); |
There was a problem hiding this comment.
I like the idea of reading the file a test to know everything works as expected, like a validation to surface potential problems.
In this initial implementation we are trying to just overwrite the value and it doesn't seem right to try to build from previous value. In any case I would like to rethink a little bit about the current ClientApiKeyDTO.
There was a problem hiding this comment.
Sure, we can look into ways of enchancing it and makint it future-proof. What do you suggest?
One thing to keep in mind, specifically about Proxy_Key we agreed that when generating a new key "previous APIs key won't be available" (see ticket). I see little point in storing APIs that cannot be used anymore.
| { | ||
| "commandName": "proxy:generatekey", | ||
| "supportsInteractiveMode": false, | ||
| "generator": "commands/proxy/generatekey/ProxyGenerateKeyCommand" |
There was a problem hiding this comment.
I would prefer to keep alphabetical order on commandName.
There was a problem hiding this comment.
Changes done. Please verify
| * | ||
| */ | ||
|
|
||
| class ClientApiKeyDTO { |
There was a problem hiding this comment.
I would like to review this class.
No description provided.