Skip to content

refactor(api): use sessionmaker in pgvecto_rs VDB service#34818

Merged
asukaminato0721 merged 1 commit intolanggenius:mainfrom
carlos4s:refactor/sessionmaker-vdb-services
Apr 9, 2026
Merged

refactor(api): use sessionmaker in pgvecto_rs VDB service#34818
asukaminato0721 merged 1 commit intolanggenius:mainfrom
carlos4s:refactor/sessionmaker-vdb-services

Conversation

@carlos4s
Copy link
Copy Markdown
Contributor

@carlos4s carlos4s commented Apr 9, 2026

Summary

  • Replace with Session(self._client) + session.commit() with
    sessionmaker(bind=self._client).begin() in pgvecto_rs VDB service (6 blocks)
  • Read-only session blocks left unchanged
  • Updated test mocks to patch both Session and sessionmaker

Part of #24245

Test plan

  • ruff check passes
  • 11 pgvecto_rs tests pass

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. refactor labels Apr 9, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-04-09 05:39:42.968336153 +0000
+++ /tmp/pyrefly_pr.txt	2026-04-09 05:39:34.492282664 +0000
@@ -163,7 +163,7 @@
 ERROR Function declared to return `bool`, but one or more paths are missing an explicit `return` [bad-return]
    --> core/rag/datasource/vdb/oracle/oraclevector.py:191:39
 ERROR No matching overload found for function `sqlalchemy.sql.sqltypes.UUID.__init__` called with arguments: (as_uuid=Literal[True]) [no-matching-overload]
-  --> core/rag/datasource/vdb/pgvecto_rs/pgvecto_rs.py:67:32
+  --> core/rag/datasource/vdb/pgvecto_rs/pgvecto_rs.py:66:32
 ERROR Argument `str | None` is not assignable to parameter `database_name` with type `str` in function `tcvectordb.rpc.client.stub.RPCVectorDBClient.describe_collection` [bad-argument-type]
   --> core/rag/datasource/vdb/tencent/tencent_vector.py:82:35
 ERROR Object of class `FilterIndex` has no attribute `dimension`

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 9, 2026
@asukaminato0721 asukaminato0721 added this pull request to the merge queue Apr 9, 2026
@asukaminato0721 asukaminato0721 requested a review from Copilot April 9, 2026 05:50
Copy link
Copy Markdown
Contributor

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

Refactors the PGVectoRS vector DB service’s write transactions to use SQLAlchemy’s sessionmaker(...).begin() context manager, aligning writes with transactional scope semantics and removing explicit commit() calls. This is part of the broader effort in #24245.

Changes:

  • Replaced multiple with Session(engine) + commit() write blocks with with sessionmaker(bind=engine).begin() in the PGVectoRS service.
  • Kept read-only session usage unchanged.
  • Updated unit tests to patch both Session and sessionmaker to match the new transaction pattern.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
api/core/rag/datasource/vdb/pgvecto_rs/pgvecto_rs.py Uses sessionmaker(...).begin() for write transactions; removes explicit commits.
api/tests/unit_tests/core/rag/datasource/vdb/pgvecto_rs/test_pgvecto_rs.py Adds test helpers/mocks for sessionmaker().begin() and patches both session entrypoints.

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

Comment on lines 57 to 60
self._client = create_engine(self._url)
with Session(self._client) as session:
with sessionmaker(bind=self._client).begin() as session:
session.execute(text("CREATE EXTENSION IF NOT EXISTS vectors"))
session.commit()
self._fields: list[str] = []
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

sessionmaker(bind=self._client) is instantiated inline in multiple methods. Consider creating a single session factory in __init__ (e.g., self._sessionmaker = sessionmaker(bind=self._client)) and then using with self._sessionmaker.begin() as session: everywhere to reduce duplication and make future configuration/testing simpler.

Copilot uses AI. Check for mistakes.
Merged via the queue into langgenius:main with commit d360929 Apr 9, 2026
31 checks passed
@carlos4s
Copy link
Copy Markdown
Contributor Author

carlos4s commented Apr 9, 2026

@asukaminato0721 Thank you very much.

@carlos4s carlos4s deleted the refactor/sessionmaker-vdb-services branch April 9, 2026 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer refactor size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants