Skip to content

fix: update SDK calls to prevent failures in E2E pipeline#179

Merged
dhruvj07 merged 3 commits intomainfrom
sdk-run-updated-providers-branch
Apr 8, 2025
Merged

fix: update SDK calls to prevent failures in E2E pipeline#179
dhruvj07 merged 3 commits intomainfrom
sdk-run-updated-providers-branch

Conversation

@dhruvj07
Copy link
Copy Markdown
Contributor

@dhruvj07 dhruvj07 commented Apr 8, 2025

update SDK calls to prevent failures in E2E pipeline

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @dhruvj07, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request addresses failures in the E2E pipeline by updating SDK calls in several example files. Specifically, it modifies the way the Javelin client is initialized and how API base URLs are constructed for Azure OpenAI and OpenAI embeddings. The changes primarily involve removing the explicit base_url parameter from the JavelinConfig and adjusting the Azure OpenAI base URL to use an environment variable.

Highlights

  • Javelin Client Initialization: The base_url parameter has been removed from the JavelinConfig in multiple example files (bedrock_client_universal.py, langchain-bedrock-universal.py, gemini-universal.py, o1-03_function-calling.py, and openai-universal.py).
  • Azure OpenAI Base URL: The Azure OpenAI base URL in azure_general_route.py has been updated to use the JAVELIN_BASE_URL environment variable instead of a hardcoded URL with a specific API version.
  • OpenAI Embeddings Endpoint: The sync_openai_embeddings function in openai_general_route.py has been modified to use a dedicated OpenAI client instance pointing to the embeddings endpoint, and the model is now text-embedding-3-small.
  • Gemini Example Updates: The gemini-universal.py example has been refactored to remove unnecessary comments and simplify the client initialization and example calls.

Changelog

Click here to see the changelog
  • examples/azure-openai/azure_general_route.py
    • Updated the Azure OpenAI base URL to use the JAVELIN_BASE_URL environment variable on line 116.
    • Removed the hardcoded API version from the base URL.
  • examples/bedrock/bedrock_client_universal.py
    • Removed the base_url parameter from the JavelinConfig on line 27.
  • examples/bedrock/langchain-bedrock-universal.py
    • Removed the base_url parameter from the JavelinConfig on line 31.
  • examples/gemini/gemini-universal.py
    • Removed unnecessary comments and simplified the init_gemini_client function on lines 10-25.
    • Removed the streaming example and refactored the main function to streamline testing on lines 38-133.
  • examples/openai/o1-03_function-calling.py
    • Removed the base_url parameter from the JavelinConfig on line 22.
  • examples/openai/openai-universal.py
    • Removed the base_url parameter from the JavelinConfig on line 42.
  • examples/openai/openai_general_route.py
    • Added a comment indicating that the client is configured for chat completions on line 22.
    • The sync_openai_completions function was removed on lines 73-84.
    • Modified sync_openai_embeddings to use a dedicated embeddings client and updated the model to text-embedding-3-small on lines 74-94.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


The pipeline's green,
SDK calls now pristine,
E2E's delight,
Code works just right,
No more failures seen!

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The pull request updates SDK calls to prevent failures in the E2E pipeline. The changes primarily involve removing the base_url parameter from the JavelinConfig initialization and updating the Azure OpenAI base URL to include the API version as a query parameter. The removal of the base URL from the Javelin config seems like it could cause issues if the base URL is not being correctly defaulted elsewhere, so I've flagged that. The removal of the streaming example from Gemini also seems odd, so I've flagged that as well.

Summary of Findings

  • Potential issue with removing base_url from JavelinConfig: The removal of the base_url parameter from the JavelinConfig initialization in multiple files might lead to unexpected behavior if the base URL is not being correctly defaulted elsewhere. This could cause the SDK calls to fail if the base URL is not properly configured.
  • Removal of Gemini streaming example: The removal of the Gemini streaming example in examples/gemini/gemini-universal.py might reduce the completeness of the example and could be confusing for users who are looking for a streaming example.
  • Inconsistent handling of API version in Azure OpenAI base URL: The Azure OpenAI base URL in examples/azure-openai/azure_general_route.py includes the API version as a query parameter, while other base URLs do not. This inconsistency might lead to confusion and potential issues with API versioning.
  • Unused openai_client parameter: The sync_openai_embeddings function in examples/openai/openai_general_route.py now takes an unused parameter, which is confusing and should be removed.

Merge Readiness

The pull request introduces changes that might have unintended consequences. The removal of the base_url parameter from JavelinConfig and the removal of the Gemini streaming example should be carefully reviewed. Additionally, the inconsistent handling of the API version in the Azure OpenAI base URL and the unused parameter in sync_openai_embeddings should be addressed. I am unable to approve this pull request, and recommend that it not be merged until these issues are addressed (at a minimum), and that others review and approve this code before merging.

@dhruvj07 dhruvj07 force-pushed the sdk-run-updated-providers-branch branch from bcf9985 to 0af6af6 Compare April 8, 2025 15:42
@dhruvj07 dhruvj07 merged commit 973fc86 into main Apr 8, 2025
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants