Fix parsing kubectl output with warnings #470
Conversation
Don't parse error output (e.g. deprecation warnings) when trying to list deployments. Signed-off-by: Usman <[email protected]>
Signed-off-by: usmonster <[email protected]>
usmonster
changed the title
|
Hello, just checking in—@davidkarlsen, @scottrigby, do you think you can take a quick look at this fix and hopefully get it into the next version? Thanks again! 🙇 |
|
I'm a bit uneasy to merge it straight as I suspect some unexpected side-effects, but I'd like @unguiculus comments as the original author. Do you remember the reasoning between getting both streams here? |
|
Maybe we should adjust |
|
Thanks @unguiculus! I like your suggestion as a separate PR, which could also help with #359. Regarding @davidkarlsen's concerns, there should be no unwanted side effects since we're now only filtering stderr output that isn't currently used or expected in these cases. Please feel free to merge if there's nothing else to change, and thanks again for the review! |
|
Just a friendly bump to @davidkarlsen & @scottrigby. 🙂 Do you need anything else from me before this can be merged? Thanks again! |
Exclude stderr (e.g. deprecation warnings) when parsing command output.
What this PR does / why we need it: It ensures that warnings or other output that goes to stderr won't be parsed when trying to list deployments, pods, etc. Today, this is not the case, and many
ctsubcommands will fail in such situations (e.g. when the gke-gcloud-auth-plugin isn't yet installed).Special notes for your reviewer:
It's possible that the original implementation wanted to include stderr in the output for these commands for a reason, but this feels like a simple oversight.
It's also possible that there are other similar places in the code that require the same modification, so please feel free to update the PR or request these changes, which I will then apply. Thanks!