Avoid crashing update-redirects when CLI output is empty #1944
Conversation
| _logger.LogInformation("Describing KeyValueStore"); | ||
| try | ||
| { | ||
| var json = CaptureMultiple("aws", "cloudfront", "describe-key-value-store", "--name", kvsName); |
There was a problem hiding this comment.
Can we have a retry mechanism here? It seems like this is a temporary error that only happens occasionally.
There was a problem hiding this comment.
Hmm... We have a retry mechanism on ExternalCommandExecutor, defaulting to 10 tries. Since we don't rethrow from it, It seems the mechanism is working. Is it worth it to have another? Each cycle can take up to 30s, accounting for the execution timeout.
I'll add individual error logging as well, that can be interesting too.
There was a problem hiding this comment.
I see.
Kinda unrelated.. but I'm starting to wonder how do we handle paging for describe-key-valiue-store?
There was a problem hiding this comment.
Paging is sort of handled partially by the AWS CLI itself. max-items determines the amount of items the CLI will output in the end, and page-size determines the internal paging in the CLI when communicating with AWS.
This means that if we had --max-items 50 --page-size 10, the CLI would call AWS up to 5 times before giving us the answer. The eTag it gives us as part of the output determines where in the query we are, and if it is null we have reached the end.
There was a problem hiding this comment.
We are now logging as warnings when exceptions occur in an execution attempt loop. They shouldn't be errors since later attempts might be successful.
…op. Emit as a warning since later executions can be successful.
Yep, if we can't reliably get the data from AWS even after all attempts, we still exit with non-zero, but cleanly. |
update-redirectsshould verify the output at each stage - an empty string is not parsed as a valid empty JSON response. We should fail the workflow step, but not crash.