Skip to content

fix: fetch http(s) type: file vector sources, reject other schemes#149

Merged
tellet-q merged 3 commits into
devfrom
fix/file-source-remote-paths
Jul 14, 2026
Merged

fix: fetch http(s) type: file vector sources, reject other schemes#149
tellet-q merged 3 commits into
devfrom
fix/file-source-remote-paths

Conversation

@tellet-q

@tellet-q tellet-q commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Allow https:// url only, reject s3://.

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?

Validation accepted `s3://` and `http(s)://` paths, but `FBinReader::new`
only opened local paths and unwrapped, so a URL panicked at upload time.

Now http(s) paths download through the dataset downloader (cached under
`$BFB_DATASETS_DIR/files/`), other schemes are rejected up front, and
`FBinReader::new` returns `Result`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tellet-q tellet-q requested a review from generall July 10, 2026 08:44
@tellet-q tellet-q marked this pull request as ready for review July 13, 2026 07:58
@tellet-q tellet-q requested a review from Copilot July 13, 2026 07:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR makes source: { type: file } vector sources safer and more flexible by supporting remote HTTP(S) .fbin files via a download+cache path, rejecting non-HTTP(S) URL schemes during config validation, and eliminating panics when opening vector files.

Changes:

  • Make FBinReader::new fallible and propagate errors instead of panicking on open/mmap failures.
  • Add ensure_local_file to download/cache HTTP(S) type: file vector sources and update generators to use it.
  • Centralize file-source path validation (including scheme rejection) and add tests/docs for remote file sources.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/upload.rs Propagates FBinReader open failures instead of panicking.
src/generators/queries.rs Resolves VectorSource::File paths via ensure_local_file before reading .fbin.
src/generators/config.rs Uses ensure_local_file for config-driven uploads and adds a test for remote .fbin vectors.
src/fbin_reader.rs Converts .fbin reader construction to Result and adds basic header-length validation.
src/dataset/mod.rs Re-exports download helpers and adds a small HTTP test server module for download-path tests.
src/dataset/download.rs Implements remote-file detection, download-to-cache for file sources, and related tests.
src/config/search.rs Switches to shared validate_file_source_path for search-config validation.
src/config/schema.rs Updates schema docs to reflect HTTP(S) file-source support (and removal of s3://).
src/config/mod.rs Adds validate_file_source_path and tests for scheme acceptance/rejection.
examples/upload-config.yaml Documents HTTP(S) URL support and caching behavior for file vector sources.
Comments suppressed due to low confidence (1)

src/fbin_reader.rs:5

  • size_of is used in this file but not imported, which will fail to compile. Import it alongside transmute (or fully-qualify the calls).
use std::fs::OpenOptions;
use std::mem::transmute;
use std::path::Path;

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

Comment thread src/fbin_reader.rs
Comment thread src/dataset/download.rs
Comment thread src/dataset/download.rs Outdated
Comment thread src/dataset/download.rs Outdated
@timvisee timvisee self-requested a review July 14, 2026 10:14
tellet-q and others added 2 commits July 14, 2026 12:37
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@tellet-q tellet-q force-pushed the fix/file-source-remote-paths branch from 1d401a6 to 4c64a97 Compare July 14, 2026 11:35
@tellet-q tellet-q merged commit a4f1096 into dev Jul 14, 2026
5 checks passed
@tellet-q tellet-q deleted the fix/file-source-remote-paths branch July 14, 2026 12:28
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.

3 participants