Skip to content

Managing environment variable for langchain project and endpoint#128

Merged
lfnothias merged 6 commits intodev_lfxfrom
dev
Feb 13, 2025
Merged

Managing environment variable for langchain project and endpoint#128
lfnothias merged 6 commits intodev_lfxfrom
dev

Conversation

@lfnothias
Copy link
Contributor

@lfnothias lfnothias commented Feb 13, 2025

PR Type

enhancement


Description

  • Improved environment variable management

  • Default project name set to "MetaboT"

  • Endpoint URL now configurable via environment


Changes walkthrough 📝

Relevant files
Enhancement
main.py
Update environment variable management logic                         

app/core/main.py

  • Added dynamic handling for LANGCHAIN_PROJECT
  • Set default project name if not provided
  • Made endpoint URL configurable via environment variable
  • +8/-5     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Contributor

    github-actions bot commented Feb 13, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit ef80e3e)

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Incorrect Environment Variable Usage

    The os.getenv function is used incorrectly with multiple arguments on lines 101 and 102. The correct usage should only include the environment variable name and an optional default value.

    LANGCHAIN_PROJECT = os.getenv("LANGCHAIN_PROJECT", "LANGSMITH_PROJECT", "MetaboT")
    LANGCHAIN_ENDPOINT = os.environ("LANGCHAIN_ENDPOINT", "LANGSMITH_ENDPOINT", "https://api.smith.langchain.com")
    Potential Misconfiguration

    The api_key retrieval on line 105 uses an incorrect default value. It should not default to "LANGSMITH_PROJECT" as this is likely a mistake.

    api_key = os.getenv("LANGCHAIN_API_KEY", "LANGSMITH_PROJECT")
    if not api_key:
        raise ValueError("LANGCHAIN_API_KEY environment variable is not set")

    @github-actions
    Copy link
    Contributor

    github-actions bot commented Feb 13, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Validate environment variable presence
    Suggestion Impact:The suggestion to raise an error if the LANGCHAIN_PROJECT environment variable is not set was partially implemented by changing the way the variable is retrieved, but it did not raise an error as suggested. Instead, it provided a default value.

    code diff:

         langchain_project = os.getenv("LANGCHAIN_PROJECT")
    -    if not langchain_project:
    -        langchain_project = "MetaboT"
    -
    -    os.environ["LANGCHAIN_ENDPOINT"] = "https://api.smith.langchain.com"
    +    LANGCHAIN_PROJECT = os.getenv("LANGCHAIN_PROJECT", "LANGSMITH_PROJECT", "MetaboT")

    Ensure that the langchain_project variable is set in the environment to avoid using
    a default value unintentionally, which could lead to incorrect project
    configurations.

    app/core/main.py [100-102]

     langchain_project = os.getenv("LANGCHAIN_PROJECT")
     if not langchain_project:
    -    langchain_project = "MetaboT"
    +    raise EnvironmentError("LANGCHAIN_PROJECT environment variable is not set.")
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion to raise an error if the LANGCHAIN_PROJECT environment variable is not set can prevent unintended configurations and ensure that the application is using the correct project settings. This change enhances robustness by avoiding default values that might not be suitable for all environments.

    Medium
    Ensure endpoint URL is configured
    Suggestion Impact:The suggestion to raise an error when the KG_ENDPOINT_URL is not set was partially implemented by changing the handling of the endpoint_url variable, but it still defaults to a URL instead of raising an error.

    code diff:

    -    endpoint_url = os.getenv("KG_ENDPOINT_URL")
    -    if not endpoint_url:
    -        endpoint_url = "https://enpkg.commons-lab.org/graphdb/repositories/ENPKG"
    -        # endpoint_url = "https://enpkg.commons-lab.org/graphdb/sparql"
    +    endpoint_url = os.getenv("KG_ENDPOINT_URL", "https://enpkg.commons-lab.org/graphdb/repositories/ENPKG")

    Validate that the endpoint_url environment variable is set to prevent defaulting to
    a potentially incorrect or outdated URL.

    app/core/main.py [211-213]

     endpoint_url = os.getenv("KG_ENDPOINT_URL")
     if not endpoint_url:
    -    endpoint_url = "https://enpkg.commons-lab.org/graphdb/repositories/ENPKG"
    +    raise EnvironmentError("KG_ENDPOINT_URL environment variable is not set.")
    Suggestion importance[1-10]: 7

    __

    Why: Raising an error when the KG_ENDPOINT_URL environment variable is not set ensures that the application does not default to a potentially incorrect or outdated URL. This change improves reliability by enforcing explicit configuration, which is crucial for connecting to the correct knowledge graph database.

    Medium

    @lfnothias
    Copy link
    Contributor Author

    /review

    @qodo-code-review
    Copy link

    Qodo Merge was enabled for this repository, but no eligible user found. Please link your Git account with your Qodo account here.

    @github-actions
    Copy link
    Contributor

    Persistent review updated to latest commit ef80e3e

    @lfnothias lfnothias merged commit 770cbfe into dev_lfx Feb 13, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants