Skip to content

fix: normalize a gRPC update_mode on the REST paths - #1605

Open
feiiiiii5 wants to merge 2 commits into
qdrant:devfrom
feiiiiii5:fix/rest-update-mode-normalization
Open

feiiiiii5 wants to merge 2 commits into
qdrant:devfrom
feiiiiii5:fix/rest-update-mode-normalization

Conversation

@feiiiiii5

Copy link
Copy Markdown
Contributor

The REST transport rejects a grpc.UpdateMode instead of converting it.

QdrantClient documents that "All methods in QdrantClient accept both gRPC and REST structures as an input. Conversion will be performed automatically." (qdrant_client.py:32-33), and types.UpdateMode is rest.UpdateMode | grpc.UpdateMode. The gRPC branches convert (qdrant_remote.py:1153, uploader/grpc_uploader.py:186), but the REST branches pass the gRPC enum straight into the REST model, where pydantic rejects it as an int:

ValidationError: update_mode
  Input should be 'upsert','insert_only' or 'update_only' [input_value=1, input_type=int]

That affects upsert, upload_points and upload_collection on a REST client, and the same value handed to RestBatchUploader directly. The three sites (and the async twins) now convert when the value is an int and not a bool; REST enums and None are left alone.

Test: pytest tests/test_uploader.py -q -k rest_transport_accepts_grpc_update_mode fails on dev with that ValidationError at qdrant_remote.py:1186 and passes here. pytest qdrant_client/local/tests tests/test_common.py tests/test_uploader.py tests/conversions -q → 274 passed. ruff format --check --line-length=99 on the four files is clean. Not run: the congruence/integration suites (they need a running server); the request bodies are checked with httpx.MockTransport.

All Submissions:

  • Contributions should target the dev branch. Did you create your branch from dev?
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Have you installed pre-commit with pip3 install pre-commit and set up hooks with pre-commit install?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

QdrantClient accepts both REST and gRPC structures and converts automatically, and the gRPC branches do convert update_mode; the REST branches passed the gRPC enum straight into the REST model, where it failed validation as an int.
@feiiiiii5
feiiiiii5 requested a review from joein as a code owner October 19, 2026 01:30
@netlify

netlify Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for poetic-froyo-8baba7 ready!

Name Link
🔨 Latest commit b20c372
🔍 Latest deploy log https://app.netlify.com/projects/poetic-froyo-8baba7/deploys/6ad5d74e3040811476d3493f
😎 Deploy Preview https://deploy-preview-1605--poetic-froyo-8baba7.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 52347421-331c-49b5-8d6c-d619066da19e
📥 Commits

Reviewing files that changed from the base of the PR and between 9f45d24 and 2e70724.

📒 Files selected for processing: 4
  • qdrant_client/async_qdrant_remote.py
  • qdrant_client/qdrant_remote.py
  • qdrant_client/uploader/rest_uploader.py
  • tests/test_uploader.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

Sync and async REST paths now convert integer update_mode values to REST update-mode models, excluding booleans. The REST uploader also performs this conversion. The gRPC upload path remains unchanged. A test checks that upsert and upload_points send models.UpdateMode.INSERT_ONLY in REST point lists when given grpc.UpdateMode.InsertOnly.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b20c3

Merge Risk: ⚪ Minimal · up to b20c3

The REST update-mode conversion is mergeable after normal checks; no specific unresolved issue is established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: converting gRPC update_mode values on REST paths.
Description check Passed The description explains the REST validation issue, affected methods, implementation, tests, and validation results. It is directly related to the changeset.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@joein joein left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @feiiiiii5

It looks good to me, but could you please simplify / remove the tests, because these ones are too complicated

@feiiiiii5

Copy link
Copy Markdown
Contributor Author

Thanks — I removed the mocked regression test as suggested. The branch now changes only the three REST conversion sites (sync/async remote and RestBatchUploader), with no test-file changes.

Re-ran the local set: pytest qdrant_client/local/tests tests/test_common.py tests/test_uploader.py tests/conversions -q → 272 passed, 1 skipped. ruff format --check --line-length=99 on the three changed source files is clean.

I can also collapse the three checks into a small helper if you prefer that shape.

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