Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always capture helm uninstall output so that errors contain meaningful info #49

Merged
merged 4 commits into from
Jan 8, 2025

Conversation

craigwalton-dsit
Copy link
Collaborator

The quiet parameter of uninstall() is designed to work the same was as the Docker Compose implementation in Inspect - the idea being that the user shouldn't see helm uninstall output in most cases (except if they ran inspect sandbox cleanup k8s or the eval was cancelled/errored).

Users have previously shared logs or screenshots of RuntimeError: Helm uninstall failed. {"release": "bmkvavf7", "result": "not captured"}. Usually this is a red herring (documented here) but the "not captured" bit is unhelpful. The reason the helm uninstall output was not captured is that the uninstall()'s quiet parameter was False so the stdout/stderr of the command will have been written directly to the process's stdout/stderr.

This PR:

  • Always captures stdout/stderr from helm uninstall subprocesses, and "forwards" it to stdout/stderr only if quiet=False
  • Improves the documentation of the uninstall function - in particular around the behaviour of quiet.

@craigwalton-dsit craigwalton-dsit force-pushed the craig/uninstall-documentation branch from dff9d26 to bb52b83 Compare January 8, 2025 16:10
@craigwalton-dsit craigwalton-dsit marked this pull request as ready for review January 8, 2025 17:11
Copy link
Collaborator

@jasongwartz-aisi jasongwartz-aisi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice improvement!

@craigwalton-dsit craigwalton-dsit merged commit 6ee9fbf into main Jan 8, 2025
5 checks passed
@craigwalton-dsit craigwalton-dsit deleted the craig/uninstall-documentation branch January 8, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants