Skip to content

Conversation

@averikitsch
Copy link
Collaborator

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

@averikitsch averikitsch requested a review from a team as a code owner February 14, 2024 00:27
@product-auto-label product-auto-label bot added the api: cloudsql-postgres Issues related to the googleapis/langchain-google-cloud-sql-pg-python API. label Feb 14, 2024
@averikitsch averikitsch requested a review from kurtisvg February 14, 2024 00:27
@kurtisvg kurtisvg self-assigned this Feb 14, 2024
Copy link
Contributor

@jackwotherspoon jackwotherspoon left a comment

Choose a reason for hiding this comment

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

@averikitsch I'm working on docs for MySQL and MSSQL so just leaving some drive by comments 😄

@kurtisvg kurtisvg assigned averikitsch and unassigned kurtisvg Feb 14, 2024
averikitsch and others added 7 commits February 14, 2024 11:46
Co-authored-by: Jack Wotherspoon <jackwoth@google.com>
Co-authored-by: Jack Wotherspoon <jackwoth@google.com>
Co-authored-by: Jack Wotherspoon <jackwoth@google.com>
Co-authored-by: Jack Wotherspoon <jackwoth@google.com>
},
"source": [
"### 🦜🔗 Library Installation\n",
"The integration lives in its own `langchain-google-cloud-sql-pg` package, so we need to install it."
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe call out that we are pip installing vertex ai for emebddings as well since you are calling this out and the next line installs two things

"source": [
"from langchain_google_cloud_sql_pg import PostgreSQLEngine\n",
"\n",
"engine.run_as_sync( # Use run_as_sync() method to run without async/await\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe move the comment "# Use run_as_sync() method to run without async/await" to the comment section above? and maybe explain why they might want to use this option

"outputs": [],
"source": [
"from langchain_google_vertexai import VertexAIEmbeddings\n",
"\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

add a text block above saying, here we choose the textembedding-gecko model, and choose the latest version. nit we should fix a version and not recommend latest.

"source": [
"from langchain_google_cloud_sql_pg import CloudSQLVectorStore\n",
"\n",
"store = CloudSQLVectorStore.create_sync(\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

is create_sync always required?

"\n",
"all_texts = [\"Apples and oranges\", \"Cars and airplanes\", \"Pineapple\", \"Train\", \"Banana\"]\n",
"metadatas = [{\"len\": len(t)} for t in all_texts]\n",
"ids = [str(uuid.uuid4()) for _ in all_texts]\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

we made it so the user has to add a id and we dont do it for them right? maybe we could a comment saying that, so that they know id will be a means they have to refer to the texts

"metadata": {},
"outputs": [],
"source": [
"from langchain_google_cloud_sql_pg.indexes import IVFFlatIndex\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add a text block, "you can create an index to speed up vector search querires. read more about indexes here"

averikitsch and others added 5 commits February 19, 2024 13:36
Co-authored-by: Jack Wotherspoon <jackwoth@google.com>
Co-authored-by: Jack Wotherspoon <jackwoth@google.com>
@averikitsch averikitsch merged commit 6c41df2 into main Feb 20, 2024
@averikitsch averikitsch deleted the akitsch-vs-doc branch February 20, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: cloudsql-postgres Issues related to the googleapis/langchain-google-cloud-sql-pg-python API.

4 participants