Stanctl-fleet: deploy and config-update#102
Conversation
| ...(tags && { tags }), | ||
| ...(groups && { groups }), | ||
| ...(args && { args }) | ||
| tags |
There was a problem hiding this comment.
Here, the code always includes tags in the request body, even when parseTags returns undefined, while deploy.ts and update.ts both handle this consistently using spread:
...(tags && { tags }),But restart.ts does not. Sending tags: undefined may cause the API to behave differently than omitting the field entirely. Fix to match the other two handlers.
| handleRestart, | ||
| handleDeploy, | ||
| handleUpdate | ||
| }); No newline at end of file |
There was a problem hiding this comment.
The diff shows \ No newline at end of file. Minor but trips linting.
| export function createAxiosInstance() { | ||
| return axios.create({ | ||
| httpsAgent: new https.Agent({ | ||
| rejectUnauthorized: false |
There was a problem hiding this comment.
Here, we disabled TLS certificate verification. At minimum, worth a comment explaining why it's intentional rather than an oversight.
| @@ -0,0 +1,87 @@ | |||
| import logger from '../logger'; | |||
There was a problem hiding this comment.
deploy.ts and update.ts are structurally identical apart from the action string and description strings, which is a strong candidate for a shared helper.
Both handlers have exactly the same structure: resolve server/token, validate, parse tags, build request, POST, log result. The only differences are:
action: 'agent.component.deploy'vs'agent.configuration.update'- The interface name
- The description strings in log output
Consider a shared sendAgentRequest(action, argv) utility that the handlers delegate to, keeping each handler file as a thin wrapper that just supplies the action string.
| }, | ||
| handlers.handleRestart | ||
| }, handlers.handleRestart) | ||
| .command( |
There was a problem hiding this comment.
Can you please double check if there's mixed indentation here? It appears the restart command block uses 4-space indentation, while the new deploy and config-update blocks use tabs. If it does, please normalize to match the rest of the file (spaces).
| ${execName} config-update --type agentType --tag key1=value1 --configurationId=configID --debug | ||
| `; | ||
|
|
||
|
|
There was a problem hiding this comment.
There are two consecutive blank lines before the export function declaration. Minor style inconsistency.
| }) | ||
| .option('token', { | ||
| alias: 't', | ||
| describe: 'API token for authenticating agent restart requests', |
There was a problem hiding this comment.
Looks like copy-paste from restart. Should be "authenticating agent fleet management requests" or something similar. The same applies to both deploy and config-update.
| * Parse an array of key=value tag strings into a Record. | ||
| * Returns undefined if the input array is empty. | ||
| */ | ||
| export function parseTags(tagsInput: string[]): Record<string, string> | undefined { |
There was a problem hiding this comment.
parseTags returns undefined for an empty array, but every caller immediately checks length === 0 before calling it. So, parseTags can never actually receive an empty array in practice. Either remove the early-return guard from parseTags and let the empty-array path be handled by callers, or remove the caller-side pre-checks and let parseTags return undefined for empty input. Right now both layers check for empty, which is redundant.
|
Please also check why DCO is failing. |
Signed-off-by: dishabhagat <disha.14624@gmail.com>
Signed-off-by: dishabhagat <disha.14624@gmail.com>
Signed-off-by: dishabhagat <disha.14624@gmail.com>
21139e8 to
978e6d6
Compare
|
Hi @morningspace, I have refactored the code. I extracted all shared logic into a single sendAgentRequest(action, argv, configurationId?) function in utils.ts, and each handler is now a thin wrapper around it. |
| const request: Record<string, any> = { | ||
| action, | ||
| type, | ||
| tags, |
There was a problem hiding this comment.
IIRC, previously deploy.ts and update.ts used ...(tags && { tags }) to conditionally include the field. Now sendAgentRequest always includes tags, even if tags is {}. Is this something intentional?
Also, for parseTags test in utils.test.ts, there's no test for what does happen when an empty array is passed which is now that parseTags returns {} (an empty object).
There was a problem hiding this comment.
Previously I had misunderstood the requirements. I thought only --configuration-id was required for deploy and config-update. I corrected that in my second commit: both --tag and --configuration-id are now always required for those commands.
Signed-off-by: dishabhagat <disha.14624@gmail.com>
2 participants
Stanctl-fleet: deploy and config-update command