Skip to content

Commit

Permalink
fix spammy output from 'eden remove'(legacy Python cli)
Browse files Browse the repository at this point in the history
Summary:
## Context

The legacy Python version of Eden's remove function can produce excessive output when using Python versions greater than 3.11. This issue arises due to a change in [the function signature of _rmtree_safe_fd from shutil](https://github.com/python/cpython/blob/3.12/Lib/shutil.py#L642C1-L642C35).

Previously, we integrated our spinner with `shutil.rmtree` in a hacky manner by decorating the inner functions and passing arguments to the spinner to print the path. However, due to the aforementioned signature change in Python 3.12, `args[0]` becomes too lengthy for printing. When this output wraps to a new line in the terminal, the `\r\033[K` trick fails to erase the text from the previous line, resulting in cluttered output.

## This Diff

To address this issue, we have implemented a new strategy to parse the arguments based on the Python version in use.

## Note

We are migrating to Rust version of this CLI command (`eden rm`) soon.

Reviewed By: MichaelCuevas

Differential Revision: D66888045

fbshipit-source-id: d7b654e0bdc3105eafc09ee426845c626a70ef92
  • Loading branch information
lXXXw authored and facebook-github-bot committed Dec 6, 2024
1 parent d9b15b0 commit 1788f4c
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 3 deletions.
41 changes: 41 additions & 0 deletions eden/fs/cli/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -930,17 +930,58 @@ def destroy_mount(
old_rmtree_safe_fd = shutil._rmtree_safe_fd

with Spinner("Deleting: ") as spinner:
# pyre-ignore[2]: Parameter must be annotated.
# pyre-fixme[3]: Return type must be annotated.
def args_parser_for_rmtree_unsafe(args):
try:
# as of Python 3.12, _rmtree_unsafe has the signature below
#
# def _rmtree_unsafe(path, onexc):
return str(args[0])
except Exception:
return "directory and files ..."

shutil._rmtree_unsafe = util.hook_recursive_with_spinner(
# We're gently caressing an internal shutil function
# pyre-ignore[16]: Module shutil has no attribute _rmtree_unsafe.
shutil._rmtree_unsafe,
spinner,
args_parser_for_rmtree_unsafe,
)

# pyre-ignore[2]: Parameter must be annotated.
# pyre-fixme[3]: Return type must be annotated.
def args_parser_for_rmtree_safe_fd(args):
try:
if sys.version_info >= (3, 12):
# as of Python 3.12, _rmtree_safe_fd has the signature below
#
# def _rmtree_safe_fd(stack, onexc):
#
# where "stack" has the structure:
# [
# (
# <built-in function rmdir>,
# None,
# PosixPath('/data/users/myUser/eden-dev-state/clients/fbsource-test-remove-4'),
# None
# )
# ]
return str(args[0][0][2])
else:
# for Python 3.11 and below, the signature is:
# def _rmtree_safe_fd(topfd, path, onerror):
return str(args[1])

except Exception:
return "directory and files ..."

shutil._rmtree_safe_fd = util.hook_recursive_with_spinner(
# We're gently caressing an internal shutil function
# pyre-ignore[16]: Module shutil has no attribute _rmtree_safe_fd.
shutil._rmtree_safe_fd,
spinner,
args_parser_for_rmtree_safe_fd,
)

path = Path(path)
Expand Down
10 changes: 7 additions & 3 deletions eden/fs/cli/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -809,21 +809,25 @@ def __exit__(self, ex_type, ex_value, ex_traceback) -> bool:


# pyre-fixme[3]: Return type must be annotated.
# pyre-fixme[24]: Generic type `Callable` expects 2 type parameters.
def hook_recursive_with_spinner(function: Callable, spinner: Spinner):
def hook_recursive_with_spinner(
function: Callable, # pyre-fixme[24]: Generic type `Callable` expects 2 type parameters.
spinner: Spinner,
args_parser: Callable, # pyre-fixme[24]: Generic type `Callable` expects 2 type parameters.
):
"""
hook_recursive_with_spinner
Hook a recursive function updating a spinner at every recursion step
Params:
- function: the recursive function to hook
- spinner: Spinner supporting text arguments
- args_parser: a callable to extract printable information from args
"""

@functools.wraps(function)
# pyre-fixme[3]: Return type must be annotated.
# pyre-fixme[2]: Parameter must be annotated.
def run(*args, **kwargs):
spinner.spin(args[0])
spinner.spin(args_parser(args))
return function(*args, **kwargs)

return run
Expand Down

0 comments on commit 1788f4c

Please sign in to comment.