-
Notifications
You must be signed in to change notification settings - Fork 119
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
docs: make pebble.Client.remove_path and Container.remove_path docs consistent #1031
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes. Couple of improvement suggestions -- feel free to tweak further.
Co-authored-by: Ben Hoyt <benhoyt@gmail.com>
Co-authored-by: Ben Hoyt <benhoyt@gmail.com>
ops/pebble.py
Outdated
@@ -2153,11 +2153,15 @@ def remove_path(self, path: str, *, recursive: bool = False): | |||
|
|||
Args: | |||
path: Path of the file or directory to delete from the remote system. | |||
recursive: If True, and path is a directory recursively deletes it and | |||
recursive: If True, and path is a directory, recursively deletes it and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also please copy the new version above to here? "recursive" and "Raises".
The mysql-k8s-operator tests changed, will open a separate PR for addressing that. |
pebble.Client.remove_path
tomodel.Container.remove_path
(and fix a missing comma).remove_path
may raisePathError
in some situations.Annoyingly the Go docs are vague on when an error will occur. The pebble code seems to raise an error if the path is relative or if the underlying Go call does. I've kept the wording fairly generic as a result.
Fixes #699