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.

Sign up for GitHub

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

Jump to bottom

Remove tenacity dependency #10822

Open
1 task done
marcinn opened this issue Jan 24, 2022 · 5 comments · May be fixed by #12705
Open
1 task done

Remove tenacity dependency #10822

marcinn opened this issue Jan 24, 2022 · 5 comments · May be fixed by #12705
Labels
project: vendored dependency Related to a vendored dependency type: refactor Refactoring code

Comments

@marcinn
Copy link

marcinn commented Jan 24, 2022 •
edited

What's the problem this feature will solve?

Tenacity dependency introduces a lot of issues, but it is used only in one pip's function. Removing this dependency will solve all class of issues with pip.

Tenacity seems to be bloated package with poor support of dependencies (see tornado issue). It is an unnecessary complication, which introduces requirement of patching and solving problems which can be simply ommited by not using it.

Describe the solution you'd like

To solve the problem change this:

@retry(reraise=True, stop=stop_after_delay(3), wait=wait_fixed(0.5))                                
     def rmtree(dir: str, ignore_errors: bool = False) -> None:                                          
         shutil.rmtree(dir, ignore_errors=ignore_errors, onerror=rmtree_errorhandler)

to something like this:

def rmtree(dir: str, ignore_errors: bool = False) -> None:
   t = time.time()
   while True:
       try:
           shutil.rmtree(dir, ignore_errors=ignore_errors, onerror=rmtree_errorhandler) 
       except Exception:
           time.sleep(0.5)
           if time.time() - t > 3:
               raise
       finally:
           break

Alternative Solutions

Additional context

Code of Conduct
@marcinn marcinn added S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature labels Jan 24, 2022
@notatallshaw
Copy link
Contributor

notatallshaw commented Jan 24, 2022 •
edited

FYI it's used in a second internal function.

The one you mentioned: https://github.com/pypa/pip/blob/21.3.1/src/pip/_internal/utils/misc.py#L125
The second one: https://github.com/pypa/pip/blob/21.3.1/src/pip/_internal/utils/filesystem.py#L100

@pradyunsg
Copy link
Member

What tornado issue are you referencing?

@pradyunsg pradyunsg added S: awaiting response Waiting for a response/more information type: refactor Refactoring code project: vendored dependency Related to a vendored dependency and removed type: feature request Request for a new feature S: needs triage Issues/PRs that need to be triaged labels Jan 24, 2022
@marcinn
Copy link
Author

marcinn commented Jan 25, 2022

AFAIK it was #10020

@no-response no-response bot removed the S: awaiting response Waiting for a response/more information label Jan 25, 2022
@ichard26
Copy link
Member

ichard26 commented May 14, 2024 •
edited

Tenacity consists of ~1600 lines of Python code which seems wasteful given a custom utility retry decorator wouldn't require more than 50 lines. Any objections if I submit a PR to replace tenacity with our own decorator? This assumes we don't plan on using tenacity's extended features like custom stop conditions or exponential backoff).

Also, FWIW, retrying which we used to use only needed ~300 lines: #9599

@ichard26
Copy link
Member

I've pushed a draft patch. I still need to write unit tests, but overall I think it's worth it: main...ichard26:pip:replace-tenacity

@ichard26 ichard26 linked a pull request May 15, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
project: vendored dependency Related to a vendored dependency type: refactor Refactoring code
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replace tenacity.retry with our own decorator ichard26/pip
4 participants
@marcinn @pradyunsg @notatallshaw @ichard26