[Partners-HuggingFace] Breaking Change - impossible to skip login after PR #23309

morgandiverrez commented 1 week ago

Checked other resources

[X] I added a very descriptive title to this issue.
[X] I searched the LangChain documentation with the integrated search.
[X] I used the GitHub search to find a similar question and didn't find it.
[X] I am sure that this is a bug in LangChain rather than my code.
[X] The bug is not resolved by updating to the latest stable version of LangChain (or the specific integration package).

Example Code

 HuggingFaceEndpoint(
            endpoint_url=self.setting.api_base,
            temperature=self.setting.temperature,
            repetition_penalty=self.setting.repetition_penalty,
            max_new_tokens=self.setting.max_new_tokens,
            server_kwargs={'trust_env': True},
        )

Error Message and Stack Trace (if applicable)

File "/home/Morgan.Diverrez/code/GitHub/tock-arkea/gen-ai/orchestrator-server/src/main/python/server/src/gen_ai_orchestrator/services/langchain/factories/llm/hugging_face_tgi_llm_factory.py", line 41, in get_language_model return HuggingFaceEndpoint( File "/home/Morgan.Diverrez/code/GitHub/tock-arkea/gen-ai/orchestrator-server/src/main/python/server/.venv/lib/python3.9/site-packages/pydantic/v1/main.py", line 341, in init raise validation_error pydantic.v1.error_wrappers.ValidationError: 1 validation error for HuggingFaceEndpoint root Did not find huggingfacehub_api_token, please add an environment variable HUGGINGFACEHUB_API_TOKEN which contains it, or pass huggingfacehub_api_token as a named parameter. (type=value_error)

Description

After PR #23309 a few months ago, when we leave huggingfacehub_api_token at its default value, None, and do not set the environment variable HF_TOKEN or HUGGINGFACEHUB_API_TOKEN, the get_from_dict_or_env function at lines 197 and 201 also has a default value of None, similar to huggingfacehub_api_token. The get_from_dict_or_env function calls get_from_env, which raises an error if no token is set in the environment variable.

Therefore, we need to use another solution to retrieve environment variables.

System Info

System Information

OS: Linux OS Version: #127~20.04.1-Ubuntu SMP Thu Jul 11 15:36:12 UTC 2024 Python Version: 3.9.18 (main, Aug 16 2024, 10:24:56) [GCC 9.4.0]

Package Information

langchain_core: 0.1.52 langchain: 0.1.20 langchain_community: 0.0.38 langsmith: 0.1.114 langchain_huggingface: 0.0.3 langchain_openai: 0.0.2.post1 langchain_text_splitters: 0.0.2

Packages not installed (Not Necessarily a Problem)

keenborder786 commented 1 week ago

I tested it on my machine and faced not issues???

morgandiverrez commented 1 week ago

here is the following detailed stack :

HuggingFaceEndpoint
- Pydantics execute the following root validator validate_environment
  - values["huggingfacehub_api_token"] = get_from_dict_or_env( values, "huggingfacehub_api_token", "HUGGINGFACEHUB_API_TOKEN", None ) it's set the default value to None
  - return get_from_env(key_for_err, env_key, default=default) that token is not defined on the constructor at the instantiation
    - elif default is not None: our default value is None, so the Error will be raised
      - else: raise ValueError( f"Did not find {key}, please add an environment variable" f" {env_key} which contains it, or pass" f" {key} as a named parameter." )

So everyone using HuggingFaceEndpoint without Hugging Face token configured in environment variable will have the same issue.

Do you have the same understanding of this stack ?

I suggest that we introduce sentinelle value as the default value in get_from_env I can push a pull request for it

Benvii commented 1 week ago

Good catch, I'm also experiencing this issue, this makes it impossible to use any custom deployment of HF TGI, that doesn't required any HF token (and login).

Introducing a sentinel value may be considered as a breaking change, get_from_env will then accept None as default value without raising any exception which is currently not accepted 🤔. This would assume that users rely on this exception being raised, which I my opinion shouldn't be considered as "nominal" usage of this method.

morgandiverrez commented 1 week ago

PR : core: [env.py/get_from_env] change default value None -> sentinel object #26153

langchain-ai / langchain