Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Clarify Python and REST API query methods in sample notebooks when re-using Reasoning engine agents #1532

Merged
merged 11 commits into from
Dec 20, 2024

Conversation

koverholt
Copy link
Member

Description

This PR clarifies the use of the Python SDK or REST API for querying a deployed Reasoning Engine instance, rather than using Vertex AI client libraries in other langauges.

  • Follow the CONTRIBUTING Guide.
  • You are listed as the author in your notebook or README file.
    • Your account is listed in CODEOWNERS for the file(s).
  • Make your Pull Request title in the https://www.conventionalcommits.org/ specification.
  • Ensure the tests and linter pass (Run nox -s format from the repository root to format).
  • Appropriate docs were updated (if necessary)

@koverholt koverholt requested a review from a team as a code owner December 13, 2024 19:10
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Contributor

@code-review-assist code-review-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.

Hi @koverholt and reviewers,

I'm currently reviewing this pull request and will post my detailed review in a few minutes. In the meantime, here's a quick summary to help everyone get up to speed:

This PR clarifies how to query a deployed Reasoning Engine instance using either the Python SDK or the REST API. The previous version of the sample notebooks incorrectly used Vertex AI client libraries for other languages. The changes specifically affect gemini/reasoning-engine/tutorial_vertex_ai_search_rag_agent.ipynb. The modifications primarily involve updating the documentation and code examples within the notebook to explicitly demonstrate the use of the Python SDK and REST API for querying the Reasoning Engine. The notebook now clearly outlines both methods, providing users with more flexibility and clarity in their interactions with the deployed agent. No functional changes were made to the core agent logic; only the instructions on how to interact with the deployed agent were updated.

Now for a little fun:

Code flows like a river,
Clear and concise, a true gift,
Reviews make it shine.

Looking forward to discussing this further!

Copy link
Contributor

@code-review-assist code-review-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.

This PR clarifies how to query a deployed Reasoning Engine agent using the Python SDK or REST API. The changes are well-structured and easy to follow. The added explanations and examples enhance the tutorial's clarity. The change in the title is also an improvement, making it more specific and informative. Overall, the changes are well-done and improve the documentation.

One potential improvement area in the original notebook is the section on "Reusing your deployed agent from other applications or SDKs". This section could be made more comprehensive by providing more detailed explanations and examples of how to query the agent using both the Python SDK and the REST API. Additionally, focusing on the resource name for reusing the agent and providing examples of REST API interaction would further enhance the tutorial's value.

@koverholt koverholt enabled auto-merge (squash) December 16, 2024 21:03
@holtskinner holtskinner disabled auto-merge December 20, 2024 16:14
@holtskinner holtskinner merged commit 45e434b into main Dec 20, 2024
9 checks passed
@holtskinner holtskinner deleted the reasoning-engine-rest-fix branch December 20, 2024 16:15
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