Skip to content

Conversation

@justyns
Copy link
Owner

@justyns justyns commented Nov 4, 2025

@justyns justyns requested a review from Copilot November 4, 2025 08:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the plugin from the legacy google-cloud-aiplatform (vertexai) SDK to the newer google-genai SDK for Google Cloud Vertex AI integration. This is a major version bump to 0.3.0.

  • Replaced vertexai SDK with google-genai SDK (version >=1.48.0)
  • Updated client initialization and API usage to new SDK patterns
  • Updated Python version requirement from >3.8 to >=3.10
  • Reordered model list and added new models (gemini-2.5-flash-lite)

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
llm_vertex.py Migrated from vertexai SDK to google-genai SDK, updated client initialization and message handling APIs
pyproject.toml Updated dependencies to google-genai>=1.48.0, bumped version to 0.3.0, and changed Python requirement to >=3.10
tests/test_llm_vertex.py Updated mocked modules to match new google-genai SDK structure
.github/workflows/test.yml Removed Python 3.9 from test matrix and fixed YAML indentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

requires-python = ">3.8"
authors = [{ name = "Justyn Shull" }]
license = { text = "Apache-2.0" }
classifiers = ["License :: OSI Approved :: Apache Software License"]
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The Python version constraint was changed from >3.8 to >=3.10. This is a breaking change that drops support for Python 3.8 and 3.9, which should be documented in the release notes or changelog to inform users.

Suggested change
classifiers = ["License :: OSI Approved :: Apache Software License"]
classifiers = ["License :: OSI Approved :: Apache Software License"]
# Breaking change: Dropped support for Python 3.8 and 3.9. Minimum required Python version is now 3.10.

Copilot uses AI. Check for mistakes.
if not conversation:
return []
messages = []
print(f"Build_history conversation: {conversation}")
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

Debug print statement should be removed or replaced with proper logging. This will clutter production output and expose internal conversation data unnecessarily.

Copilot uses AI. Check for mistakes.
chat = self.client.chats.create(
model=self.model_name,
config=config,
history=history if history else None
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The conditional history if history else None is redundant since build_history already returns an empty list when there's no conversation. Either pass history directly or update the API call to handle empty lists appropriately.

Suggested change
history=history if history else None
history=history

Copilot uses AI. Check for mistakes.
@avoidik
Copy link
Contributor

avoidik commented Jan 9, 2026

I added it in my fork some time ago. https://github.com/avoidik/llm-vertex

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.

deprecation warning.

3 participants