Disable tqdm progress bar if no TTY attached

[mssalvatore]

When dockerized applications write to STDOUT/STDERR, the applications can block due to logging back pressure (see
https://docs.docker.com/config/containers/logging/configure/#configure-the-delivery-mode-of-log-messages-from-container-to-log-driver6

HuggingFace's TGI container is one such example (see huggingface/text-generation-inference#1186).

Setting tqdm's disable=None will disable the progress bar if no tty is attached and help to resolve TGI's issue #1186.

References:
huggingface/text-generation-inference#1186 (comment)
huggingface/text-generation-inference#1186 (comment)

[Wauplin]

Hi @mssalvatore, thanks for investigating this issue! Just to be sure, the solution for TGI would be to disable progress bars when downloading? For what I understand the PR simply change that if logger.getEffectiveLevel() != logging.NOTSET then the disable value is set to None instead of False. Is that the expected fix?

For your info, disabling progress bars can be done by setting HF_HUB_DISABLE_PROGRESS_BARS=1 as an environment variable. Maybe that could be a temporary fix for you before figuring out at which level we want to fix this (I'm still not so sure if this is a huggingface_hub bug).

pinging @OlivierDehaene @Narsil who maintain TGI

[mssalvatore]

@Wauplin, yes, that's the intended fix. It preserves the current behavior which is that the progress bar is disabled if logger.getEffectiveLevel() != logging.NOTSET. When the disable value is set to None, the bar is disabled if no TTY is attached. I'll give HF_HUB_DISABLE_PROGRESS_BARS a try and see if it resolves the issue.

Regarding whether or not huggingface_hub should accept this patch: is there a reason you would ever want the progress bar to be enabled if no TTY were attached?

[Wauplin]

TIL from tqdm's docs:

So yes, definitely agree with this PR! I wasn't aware of the nuance between None and False is this particular case. Thanks for noticing!

[Wauplin]

Thanks @mssalvatore for finding out about this and opening this PR! The change looks good to me now (didn't know about this default behavior before). Could you also update the tqdm in lfs.py as well (same fix + same comment). Thanks in advance!

[HuggingFaceDocBuilderDev]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

[Wauplin]

Thanks for making the changes @mssalvatore ! Failed tests in CI are unrelated so let's get it merge :)

[lllyasviel]

This just wasted me 3 hours to debug what caused by progress bar disappeared. Whoever merge this must be completely nonsense. Please do consider revert this as soon as possible!

[Wauplin]

Whoever merge this must be completely nonsense. Please do consider revert this as soon as possible!

Hi, merger here 👋 What was your setup exactly and what solution would you suggest to address the issue described in the PR description?

[juhyeon-cha]

Whoever merge this must be completely nonsense. Please do consider revert this as soon as possible!

Hi, merger here 👋 What was your setup exactly and what solution would you suggest to address the issue described in the PR description?

Hugging Face already has a feature to disable tqdm by name. Is there a need to separately disable based on TTY status?
If someone wants to see the progress bar in a non-interactive environment, there is currently no way to do so.

https://github.com/tqdm/tqdm?tab=readme-ov-file#faq-and-known-issues
I seem to have misunderstood. It appears to be related to a different issue.