Use HF_HUB_OFFLINE + fix has_file in offline mode

[Wauplin]

Should fix #30345.
Related to #31010 as well, even though it makes sense to merge both PRs IMO.

This PR:

• delegates offline mode to huggingface_hub with HF_HUB_OFFLINE env variable instead of TRANSFORMERS_OFFLINE. This change is still backward-compatible meaning that users that had already set TRANSFORMERS_OFFLINE won't notice a difference. The goal is to have a single environment variable to disable network in all HF libraries.
• updates the docs/tests to showcase HF_HUB_OFFLINE env variable
• make sure to respect offline mode in has_file => it was not the case before, causing Problem with pretrained Transformers in Offline mode  #30345 and Do not trigger autoconversion if local_files_only #31004
• add local_files_only and cache_dir to has_file() helper. At the moment, if connection/network is disabled, an error is raised. But if an authentication/authorization problem occurs, it returns False. IMO this is not consistent and we should aim at having the same response if server can't be requested. Instead of returning False, I suggest we default to checking the cache directory in case the information exists there.
• adds a test to check has_file in offline mode

Please let me know what you think.

(Note: some tests are failing but I prefer to get theoretical approval before moving forward on this).

[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.

[amyeroberts]

Thanks for working on this!

Overall I think it looks good - only thing is that I think we need a deprecation cycle for TRANSFORMERS_OFFLINE

[Wauplin]

only thing is that I think we need a deprecation cycle for TRANSFORMERS_OFFLINE

Agree but if we want to deprecate it, I think we should do it in huggingface_hub directly. As a first step, let's hide it, then I'll take care of removing it from where it's used in our services (inference api for instance) and then we can think of a deprecation cycle for it. In any case I don't think we'll remove it before transformers v5, right?

[amyeroberts]

Agree but if we want to deprecate it, I think we should do it in huggingface_hub directly.

OK - sounds good to me!

In any case I don't think we'll remove it before transformers v5, right?

I don't know, tbh, probably not before v5 as it's a common argument but if we think that's going to be too far away we could always move if forward

[marianpascalau]

Should fix #30345. Related to #31010 as well, even though it makes sense to merge both PRs IMO.

This PR:

• delegates offline mode to huggingface_hub with HF_HUB_OFFLINE env variable instead of TRANSFORMERS_OFFLINE. This change is still backward-compatible meaning that users that had already set TRANSFORMERS_OFFLINE won't notice a difference. The goal is to have a single environment variable to disable network in all HF libraries.
• updates the docs/tests to showcase HF_HUB_OFFLINE env variable
• make sure to respect offline mode in has_file => it was not the case before, causing Problem with pretrained Transformers in Offline mode  #30345 and Do not trigger autoconversion if local_files_only #31004
• add local_files_only and cache_dir to has_file() helper. At the moment, if connection/network is disabled, an error is raised. But if an authentication/authorization problem occurs, it returns False. IMO this is not consistent and we should aim at having the same response if server can't be requested. Instead of returning False, I suggest we default to checking the cache directory in case the information exists there.
• adds a test to check has_file in offline mode

Please let me know what you think.

(Note: some tests are failing but I prefer to get theoretical approval before moving forward on this).

I want to make a suggestion. In order to better follow the implementation in "huggingface_hub", instead of following the envVariable you better follow the boolean variable (with the same name) from withing huggingface_hub module:

from huggingface_hub.constants import HF_HUB_OFFLINE

if HF_HUB_OFFLINE:
    print("Now we are offline")

This will better follow the huggingface implementation when they decide to change something.

[Wauplin]

Hi @marianpascalau, thanks for the suggestion! Yes, that's exactly what I'm doing here to delegate the env variable handling to huggingface_hub.

[ydshieh]

Hi @Wauplin Is this PR also trying to fix the currently failing tests seen in other PRs too?

python3 -m pytest -v tests/utils/test_offline.py::OfflineTests::test_offline_mode

            # Force fetching the files so that we can use the cache
            mname = "hf-internal-testing/tiny-random-bert"
            BertConfig.from_pretrained(mname)
            BertModel.from_pretrained(mname)
            BertTokenizer.from_pretrained(mname)
            pipeline(task="fill-mask", model=mname)
    
            # baseline - just load from_pretrained with normal network
            cmd = [sys.executable, "-c", "\n".join([load, run, mock])]
    
            # should succeed
            env = self.get_env()
            # should succeed as HF_HUB_OFFLINE=1 tells it to use local files
            env["HF_HUB_OFFLINE"] = "1"
            result = subprocess.run(cmd, env=env, check=False, capture_output=True)
>           self.assertEqual(result.returncode, 0, result.stderr)
E           AssertionError: 1 != 0 : b'2024-05-27 10:37:27.081924: I tensorflow/tsl/cuda/cudart_stub.cc:28] Could not find cuda drivers on your machine, GPU will not be used.\n2024-05-27 10:37:27.138381: I tensorflow/core/platform/cpu_feature_guard.cc:182] This TensorFlow binary is optimized to use available CPU instructions in performance-critical operations.\nTo enable the following instructions: AVX2 FMA, in other operations, rebuild TensorFlow with the appropriate compiler flags.\n2024-05-27 10:37:28.148684: W tensorflow/compiler/tf2tensorrt/utils/py_utils.cc:38] TF-TRT Warning: Could not find TensorRT\nTraceback (most recent call last):\n  File "<string>", line 7, in <module>\n  File "/transformers/src/transformers/modeling_utils.py", line 3458, in from_pretrained\n    raise EnvironmentError(\nOSError: hf-internal-testing/tiny-random-bert does not appear to have a file named pytorch_model.bin, model.safetensors, tf_model.h5, model.ckpt or flax_model.msgpack.\n'

[Wauplin]

Hi @ydshieh I think #31010 should fix it (but maybe this one as well). Both are kinda related

[ydshieh]

Thanks! I will check

[LysandreJik]

Thanks @Wauplin ! To ensure that the deprecation cycle is respected, could we keep a test with TRANSFORMERS_OFFLINE while ensuring that it throws a warning?

As @amyeroberts said, this was a very well advertised argument

[Wauplin]

Yes @LysandreJik @amyeroberts completly right. I reverted the tests to use TRANSFORMERS_OFFLINE (then no breaking change compared to main). And I added one to test that HF_HUB_OFFLINE is respected as well.

[amyeroberts]

Thanks for handling this!

[Wauplin]

Looks like I broke some tests + have merge conflicts. Let me handle those and I'll ping you for final review.

[Wauplin]

@amyeroberts Tests are passing now. Feel free to do a last review or merge when it's ready :)

[311-code]

I would also like to see added to hub.py's def send_telemetry something like:

def redact_sensitive_info(value): # Redact sensitive information such as paths or tokens if isinstance(value, str) and (os.path.exists(value) or 'token' in value.lower()): return "[REDACTED]" return value

because how it is now seems like a bit of a security issue to me with the:

        args_as_dict = {k: v for k, v in args.__dict__.items() if not k.startswith("_") and v is not None}
        if "model_name_or_path" in args_as_dict:
            model_name = args_as_dict["model_name_or_path"]

and

 if "dataset_name" in args_as_dict:
            data["dataset_name"] = args_as_dict["dataset_name"]

I noticed there were some bert models in 2022 trained on imdb (for potential token ablation research I'm theorizing?) and really hope this is not collecting training data on celebrity string names or other things... It only seems to filter _ and None.

Ps. Can someone also explain how the example strings docs and telemetry work? Very confused there on what the telemetry is sending if for instance I use stable_video_diffusion_pipeline.py and example strings doc I see is present in that file.

[Wauplin]

Hi @311-code, concerns are legit but could you open a new issue on Github for it? This PR is related to offline mode only (and just so you know, when offline mode is enabled, telemetry is automatically stopped)

[311-code]

Yes i won't be in front of PC until late tonight but I will highlight things I noticed. I am not quite sure if the redact code will work or have side effects yet.