Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

Fix sandboxctl destroy timeout #743

Merged
merged 7 commits into from
Jun 17, 2021
Merged

Fix sandboxctl destroy timeout #743

merged 7 commits into from
Jun 17, 2021

Conversation

Ayelet41
Copy link
Contributor

@Ayelet41 Ayelet41 commented Jun 9, 2021

Added run_command_interactive to solve sandboxctl destroy hang without affecting other call to run_command function.

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 9, 2021
@Ayelet41 Ayelet41 linked an issue Jun 9, 2021 that may be closed by this pull request
@Ayelet41 Ayelet41 added the python Pull requests that update Python code label Jun 10, 2021
sre-recipes/recipe.py Outdated Show resolved Hide resolved
terraform/destroy.sh Outdated Show resolved Hide resolved
sre-recipes/sandboxctl Show resolved Hide resolved
terraform/destroy.sh Outdated Show resolved Hide resolved
Copy link
Member

@daniel-sanche daniel-sanche left a comment

Choose a reason for hiding this comment

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

Feedback on the warning message

Ayelet41 added 2 commits June 16, 2021 11:17
Use describe instead of list to reduce waiting time.
)
output, error = process.communicate()
return output, error
process = subprocess.run(command.split())
Copy link
Member

@daniel-sanche daniel-sanche Jun 17, 2021

Choose a reason for hiding this comment

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

nit: It might be useful to return some data from the completed process like before. If not, then you should get rid of the unused process variable

Copy link
Member

@daniel-sanche daniel-sanche left a comment

Choose a reason for hiding this comment

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

Tested it and it's working for me now, thanks for putting this together!

@Ayelet41 Ayelet41 merged commit 21bb114 into master Jun 17, 2021
@daniel-sanche daniel-sanche mentioned this pull request Jun 25, 2021
@minherz minherz deleted the fix_sandboxctl_timeout branch February 7, 2023 16:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement. python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sandboxctl destroy timeout
4 participants