Skip to content

fix: expose Qdrant https option#5380

Merged
kartik-mem0 merged 1 commit into
mem0ai:mainfrom
he-yufeng:fix/qdrant-https-option
Jun 12, 2026
Merged

fix: expose Qdrant https option#5380
kartik-mem0 merged 1 commit into
mem0ai:mainfrom
he-yufeng:fix/qdrant-https-option

Conversation

@he-yufeng

@he-yufeng he-yufeng commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add an explicit https option to the Qdrant vector-store config
  • pass that option through to QdrantClient
  • document the Python config option for self-hosted HTTP Qdrant with API-key auth
  • cover both config parsing and client construction with focused unit tests

Closes #5378

To verify

  • python -m pytest tests\vector_stores\test_qdrant_config.py -q
  • python -m py_compile mem0\configs\vector_stores\qdrant.py mem0\vector_stores\qdrant.py tests\vector_stores\test_qdrant_config.py
  • python -m ruff check mem0\configs\vector_stores\qdrant.py mem0\vector_stores\qdrant.py tests\vector_stores\test_qdrant_config.py
  • git diff --check

Notes

ruff format --check would reformat existing unrelated lines in mem0/vector_stores/qdrant.py, so I did not run a full-file format pass for this small compatibility fix.

@CLAassistant

CLAassistant commented Jun 5, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@he-yufeng

Copy link
Copy Markdown
Contributor Author

The remaining red Vercel status is the repository-side preview authorization gate, not a build or test failure from this branch. The status target is �ercel.com/git/authorize for the Mem0 team, which I cannot authorize from the fork.

No branch changes from me here unless a code-owned check fails or a maintainer wants the preview disabled for this PR.

@kartik-mem0

Copy link
Copy Markdown
Contributor

Changes Requested

Strengths

  • mem0/configs/vector_stores/qdrant.py:19Optional[bool] = Field(None, ...) is the right type choice: preserves the three-way distinction (force-on, force-off, unset/let client decide). Exact match for how api_key, url, and host are declared.
  • mem0/vector_stores/qdrant.py:68–69if https is not None: params["https"] = https guard correctly avoids injecting False when the user hasn't configured anything, so existing deployments are unaffected.
  • tests/vector_stores/test_qdrant_config.pytest_qdrant_passes_explicit_https_to_client monkeypatches QdrantClient at module level and asserts the exact kwargs, including https=False. That's the right way to verify constructor forwarding without a live Qdrant instance.

Issues

Important (should fix)

  1. docs/components/vectordbs/dbs/qdrant.mdx — The Qdrant config reference table (lines 69–79) lists every parameter by name (host, port, path, url, api_key, on_disk, …). The new https field is absent. Users who scan the docs to configure a self-hosted HTTP instance with API key auth — exactly the audience this fix targets — will have no idea the option exists. Please add a row: https / Whether to force HTTPS on or off. None lets the client decide (it defaults to HTTPS when an API key is supplied). Set False to connect over plain HTTP. / None.

Minor (optional)

  1. mem0/vector_stores/qdrant.py — The if https is not None: block sits outside the if host and port: block, so https is also forwarded when using url=. That's actually fine — QdrantClient accepts https alongside a URL, though the URL's embedded scheme takes precedence. The Field description says "for host/port Qdrant connections," which implies it only applies to that mode. Worth a small clarification: "applies to all connection modes; when a full URL with an explicit scheme is provided, the URL scheme takes precedence."

  2. No linked issue in the PR description — the contributing guide asks for Closes #N or an explanation. There are related issues (e.g. Qdrant vector store uses HTTPS unexpectedly with host/port/api_key for self-hosted HTTP Qdrant #5378) worth linking.

Duplicate cluster note

There are three other open PRs addressing the same thing: #5390, #5397, and #5400. #5390 already includes the docs update and additional tests, so if it's otherwise clean it would be the one to merge. If #5390 has problems, this PR is the tidiest base — the one docs line above would make it complete.

Evidence

  • CI: green (Vercel is an infra auth failure, not a code check — ignored; CLA signed)
  • Targeted tests: hatch -e dev_py_3_12 run pytest tests/vector_stores/test_qdrant_config.py -v --tb=short → 2 passed in 3.73s
  • Ruff: all checks passed on the three changed files
  • Red-green proof: N/A — config enhancement, not a bug fix
  • Could NOT verify: live QdrantClient behavior with https=False + API key on a plain-HTTP server — code tracing only; the constructor forwarding is confirmed by the mock test

Assessment

Ready to merge: With fixes — the implementation and tests are solid; the one blocker is the missing docs row. Please add it (and optionally link the related issue) and this is ready to squash.

@he-yufeng he-yufeng force-pushed the fix/qdrant-https-option branch from 0c0837c to e96f7f2 Compare June 12, 2026 13:15
@he-yufeng

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review. I pushed an update that addresses the blocker here:

  • added the Python https row in docs/components/vectordbs/dbs/qdrant.mdx
  • clarified the config/docstring wording so it doesn't imply host/port-only behavior; an explicit URL scheme still takes precedence
  • updated the PR description with Closes #5378

I also re-ran the focused checks locally:

  • python -m pytest tests\vector_stores\test_qdrant_config.py -q -> 2 passed
  • python -m py_compile mem0\configs\vector_stores\qdrant.py mem0\vector_stores\qdrant.py tests\vector_stores\test_qdrant_config.py
  • python -m ruff check mem0\configs\vector_stores\qdrant.py mem0\vector_stores\qdrant.py tests\vector_stores\test_qdrant_config.py
  • git diff --check

All passed. I saw the duplicate cluster as well; this branch now keeps the narrower constructor-forwarding test coverage, so I'll leave the merge choice to maintainers rather than adding more churn.

@kartik-mem0

Copy link
Copy Markdown
Contributor

Verified against e96f7f2.

LGTM — ready for human review & squash-merge

What this PR does

Exposes the https parameter in QdrantConfig and Qdrant.__init__, forwarding it to QdrantClient so users can force HTTP for plain-HTTP Qdrant instances that also require an API key.

Strengths

  • https: Optional[bool] = Field(None, ...) is the correct three-way distinction — the if https is not None: guard prevents accidental override
  • mem0/configs/vector_stores/qdrant.py follows the exact Pydantic v2 provider field pattern
  • mem0/vector_stores/qdrant.py — forwarding sits outside if host and port:, correctly applying to all connection modes
  • tests/vector_stores/test_qdrant_config.py — monkeypatching at module level is the right approach here given how the existing test class is structured
  • docs/components/vectordbs/dbs/qdrant.mdxhttps row is now present between api_key and on_disk, description is accurate and user-friendly

Issues

None.

Evidence

  • CI: Vercel fail is infra noise (authorization redirect, same as prior run); license/cla pass. All real package CIs green.
  • Targeted tests: hatch -e dev_py_3_12 run pytest tests/vector_stores/test_qdrant_config.py -v --tb=short → 2 passed in 0.69s (new head)
  • Ruff: ruff check mem0/configs/vector_stores/qdrant.py mem0/vector_stores/qdrant.py tests/vector_stores/test_qdrant_config.py → all checks passed
  • Red-green proof: N/A — config enhancement, not a bug fix
  • Docs: docs/components/vectordbs/dbs/qdrant.mdx line 79 — https row confirmed present in the Python parameter table
  • Linked issue: Closes #5378 now referenced in PR description
  • Cross-package: TS counterpart unaffected — https is Python-only behavior; no parity gap
  • Duplicate cluster note: fix(qdrant): add https option to disable auto-detect for self-hosted instances #5390 (ProgrammerPlus1998) is also open with the same fix. Both PRs are now complete. A maintainer should merge one and close the other — this PR is narrower and cleaner; fix(qdrant): add https option to disable auto-detect for self-hosted instances #5390 adds more config tests. Either is mergeable; the choice is yours.
  • Could NOT verify: live Qdrant behavior (plain HTTP + API key path) — code tracing only

Assessment

Ready to merge: Yes (squash) · Est. human time: ~2 min
Implementation is correct, tests verify the exact kwargs path, and the docs gap that blocked the previous review is resolved.

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.

Qdrant vector store uses HTTPS unexpectedly with host/port/api_key for self-hosted HTTP Qdrant

3 participants