skyl / corpora

Corpora is a self-building corpus that can help build other arbitrary corpora
GNU Affero General Public License v3.0
2 stars 0 forks source link

feat(corpora_ai): support azure_endpoint for openai as well #56

Closed skyl closed 3 days ago

skyl commented 3 days ago

PR Type

Enhancement, Tests


Description


Changes walkthrough ๐Ÿ“

Relevant files
Enhancement
provider_loader.py
Add support for Azure endpoint in OpenAI client                   

py/packages/corpora_ai/provider_loader.py
  • Added support for azure_endpoint in the OpenAI client.
  • Improved handling of environment variables for Azure OpenAI.
  • +7/-4     
    llm_client.py
    Implement AzureOpenAI client for Azure endpoint handling 

    py/packages/corpora_ai_openai/llm_client.py
  • Introduced AzureOpenAI client for handling Azure endpoints.
  • Conditional logic to choose between OpenAI and AzureOpenAI.
  • +11/-2   
    Tests
    test_provider_loader.py
    Update tests for Azure endpoint support                                   

    py/packages/corpora_ai/test_provider_loader.py
  • Updated test to include azure_endpoint parameter.
  • Ensured mock client is called with the new parameter.
  • +5/-1     

    ๐Ÿ’ก PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    github-actions[bot] commented 3 days ago

    PR Reviewer Guide ๐Ÿ”

    (Review updated until commit https://github.com/skyl/corpora/commit/c105ebda84ed2f84886b3c05256c7bdc2db4028d)

    Here are some key observations to aid the review process:

    โฑ๏ธ Estimated effort to review: 3 ๐Ÿ”ต๐Ÿ”ต๐Ÿ”ตโšชโšช
    ๐Ÿงช PR contains tests
    ๐Ÿ”’ No security concerns identified
    โšก Recommended focus areas for review

    Code Smell
    The use of `os.getenv("OPENAI_AZURE_ENDPOINT", None)` could be improved by explicitly checking for the presence of the environment variable and handling potential issues with its value. Code Smell
    The comment about not pinning the API version in the AzureOpenAI client initialization raises concerns about potential future compatibility issues. Consider clarifying or addressing this comment.
    github-actions[bot] commented 3 days ago

    PR Code Suggestions โœจ

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Verify the success of the build process before uploading the artifact ___ **Add a step to verify the success of the build process before uploading the artifact
    to ensure that only successful builds are stored.** [.github/workflows/ci-xcompile.yml [47-51]](https://github.com/skyl/corpora/pull/56/files#diff-33947c0b61f76a2ddc086f205419435e63997616ce75475cd0d9efbae40b8988R47-R51) ```diff - name: Build the project working-directory: rs run: | rustup target add ${{ matrix.target }} cargo build --release --target ${{ matrix.target }} + if [ $? -ne 0 ]; then exit 1; fi ```
    Suggestion importance[1-10]: 8 Why: Adding a check to verify the success of the build process before uploading the artifact is a valuable enhancement. It ensures that only successful builds are stored, which can prevent issues with deploying faulty binaries.
    8
    Possible issue
    Validate the azure_endpoint before using it to initialize the AzureOpenAI client ___ **Ensure that the azure_endpoint is validated before using it to initialize the
    AzureOpenAI client to prevent potential misconfigurations or runtime errors.** [py/packages/corpora_ai_openai/llm_client.py [20-25]](https://github.com/skyl/corpora/pull/56/files#diff-de8da8414122015059375c328610dcc9a2d9550504ca03bfaa97ec5eed468407R20-R25) ```diff -if azure_endpoint: +if azure_endpoint and is_valid_endpoint(azure_endpoint): self.client = AzureOpenAI( api_key=api_key, azure_endpoint=azure_endpoint, # What's the behavior of not pinning the API version? # api_version="2024-10-01-preview", ) ```
    Suggestion importance[1-10]: 7 Why: The suggestion to validate the `azure_endpoint` before using it can prevent potential misconfigurations or runtime errors, enhancing the robustness of the code. However, the implementation of `is_valid_endpoint` is not provided, which slightly reduces the immediate applicability of the suggestion.
    7
    skyl commented 3 days ago

    cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR

    Could not find openssl via pkg-config:

    pkg-config exited with status code 1

    PKG_CONFIG_PATH=/usr/lib/aarch64-linux-gnu/pkgconfig/: PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 pkg-config --libs --cflags openssl

    The system library openssl required by crate openssl-sys was not found. The file openssl.pc needs to be installed and the PKG_CONFIG_PATH environment variable must contain its parent directory. PKG_CONFIG_PATH contains the following:

    • /usr/lib/aarch64-linux-gnu/pkgconfig/
    • HINT: you may need to install a package such as openssl, openssl-dev or openssl-devel.

      cargo:warning=Could not find directory of OpenSSL installation, and this -sys crate cannot proceed without this knowledge. If OpenSSL is installed and this crate had trouble finding it, you can set the OPENSSL_DIR environment variable for the compilation process. See stderr section below for further information.

      --- stderr

      Could not find directory of OpenSSL installation, and this -sys crate cannot proceed without this knowledge. If OpenSSL is installed and this crate had trouble finding it, you can set the OPENSSL_DIR environment variable for the compilation process.

      Make sure you also have the development packages of openssl installed. For example, libssl-dev on Ubuntu or openssl-devel on Fedora.

      If you're in a situation where you think the directory should be found automatically, please open a bug at https://github.com/sfackler/rust-openssl and include information about your system as well as this message.

      $HOST = x86_64-unknown-linux-gnu $TARGET = aarch64-unknown-linux-gnu openssl-sys = 0.9.104

    warning: build failed, waiting for other jobs to finish... Error: Process completed with exit code 101.

    skyl commented 3 days ago

    /describe

    skyl commented 3 days ago

    /review

    github-actions[bot] commented 3 days ago

    PR Description updated to latest commit (https://github.com/skyl/corpora/commit/c105ebda84ed2f84886b3c05256c7bdc2db4028d)

    github-actions[bot] commented 3 days ago

    Persistent review updated to latest commit https://github.com/skyl/corpora/commit/c105ebda84ed2f84886b3c05256c7bdc2db4028d