Skip to content

urlencode collection_name on calling partial snapshot create API#7782

Merged
generall merged 1 commit intodevfrom
urlencode-partial-snapshot
Dec 15, 2025
Merged

urlencode collection_name on calling partial snapshot create API#7782
generall merged 1 commit intodevfrom
urlencode-partial-snapshot

Conversation

@KShivendu
Copy link
Member

@KShivendu KShivendu commented Dec 15, 2025

Otherwise partial snapshots on test%20collection fails.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 15, 2025

📝 Walkthrough

Walkthrough

This pull request adds the urlencoding dependency to the workspace and applies URL encoding to collection names in the snapshot API endpoint construction. The recover_partial_snapshot_from function is modified to encode the collection name using urlencoding::encode() before inserting it into the URL path, replacing the previous direct string insertion.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single dependency addition with minimal scope
  • Straightforward string encoding logic with no control flow changes
  • Limited file impact (one source file modified beyond Cargo.toml)
  • Homogeneous change following a clear pattern

Possibly related PRs

Suggested reviewers

  • coszio
  • xzfc

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title accurately describes the main change: URL-encoding the collection_name parameter when calling the partial snapshot creation API.
Description check ✅ Passed The description is related to the changeset, explaining the issue being fixed: partial snapshots failing for collections with encoded names like 'test%20collection'.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch urlencode-partial-snapshot
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e91702 and 637a375.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • Cargo.toml (1 hunks)
  • src/actix/api/snapshot_api.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (.github/review-rules.md)

**/*.rs: Prefer explicit SomeType::from(x) over implicit x.into() in Rust
In new code, don't use transmute_from_u8, transmute_to_u8, transmute_from_u8_to_slice, transmute_from_u8_to_mut_slice, transmute_to_u8_slice - use bytemuck or zerocopy crates instead
Prefer explicit exhaustive matches over catch-all _ arm in Rust match expressions, so we don't miss handling new enum variants when they are added later (exceptions: tests and benchmarks, or when sure new variants won't affect logic)
Prefer explicit field ignoring using : _ over using .. in Rust struct destructuring (exceptions: tests and benchmarks, or when sure new fields won't affect logic)

Files:

  • src/actix/api/snapshot_api.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: test-consistency
  • GitHub Check: integration-tests-consensus
  • GitHub Check: integration-tests
  • GitHub Check: test-shard-snapshot-api-s3-minio
  • GitHub Check: test-consensus-compose
  • GitHub Check: e2e-tests
  • GitHub Check: Build Qdrant Edge Python bindings
  • GitHub Check: rust-tests (ubuntu-latest)
  • GitHub Check: rust-tests (macos-latest)
  • GitHub Check: rust-tests (windows-latest)
  • GitHub Check: lint
🔇 Additional comments (2)
src/actix/api/snapshot_api.rs (1)

737-739: URL encoding implementation is correct.

The fix properly encodes the collection name before embedding it in the URL path. This resolves the issue with collection names containing special characters like spaces (which become %20). The urlencoding::encode() function correctly implements percent-encoding per RFC 3986.

Cargo.toml (1)

77-77: Dependency addition looks appropriate.

The urlencoding crate is the standard choice for URL encoding in Rust. Version 2.1.3 (defined in workspace dependencies at line 232) is the latest version with no known security advisories.

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 and usage tips.

@generall generall merged commit 5b71996 into dev Dec 15, 2025
15 checks passed
@generall generall deleted the urlencode-partial-snapshot branch December 15, 2025 23:16
timvisee pushed a commit that referenced this pull request Dec 18, 2025
@KShivendu @timvisee
urlencode collection_name on calling partial snapshot create API (#7782)
cdcea13
@timvisee timvisee mentioned this pull request Dec 19, 2025
Merged
@claude claude bot mentioned this pull request Jan 9, 2026
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@generall generall generall approved these changes

@timvisee timvisee Awaiting requested review from timvisee

@ffuugoo ffuugoo Awaiting requested review from ffuugoo

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@KShivendu @generall