Fix federated chunk fetch: URL residue + project-scope routing (todo #153) - #272
Merged
Merged
Conversation
…ting (todo #153) Two independent defects in remote chunk fetch: 1. The peer URL replaced '{id' without the closing brace, producing /chunk/2058%7D — axum's {id} param swallowed the stray '}' into the captured value, so mock-based tests passed while real peers answered 400. Replace the full '{id}' placeholder; pinned by a test whose route echoes the exact path it was hit on. 2. get_chunk(project=<peer>/<alias>, chunk_id) died with 'Unknown alias': mounted remote projects now route through the same federated fetch search uses (local aliases win name clashes). Hermetic routing test (temp config file — the in-memory config is reloaded from disk, which previously leaked the developer's real peers into the test).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remote chunk fetch was broken in multi-repo serve mode (todo #153). Two independent defects:
1. Stray
%7Din the peer URL (chunk_refpath)The URL was built with
CHUNK_PATH.replace("{id", …)— missing the closing brace. WithCHUNK_PATH = "/chunk/{id}"the result was/chunk/2058}(wire:%7D), which peers answer 400. Introduced by the axum 0.8 route-syntax migration (#256): the old.replace(":id", …)became.replace("{id", …)but the template grew a}.Why tests missed it: axum's
{id}parameter happily captures2058}including the stray brace, so every mock-based test matched. The new test's route echoes the exact path it was hit on (/chunk/2058), so any placeholder residue fails.2.
project=<peer>/<alias>+chunk_id→ "Unknown alias"get_chunkonly supported thechunk_refform for remote fetches; a plainproject=mount fell into local routing and died.project=mounts now route through the samefederated_get_chunkpath search uses (mounted projects only — local aliases always win a name clash), via a synthetic<peer>/<alias>:<id>ref.Tests
get_chunk_requests_the_exact_chunk_path_without_placeholder_residue— fails without fix 1get_chunk_routes_mounted_remote_projects_through_federation— fails without fix 2; hermetic (temp config file — discovered the hard way thatServeState::new(config, None)gets its config reloaded from the real disk file, which made the first test run hit a production peer)Changelog
### Fixedentry under the pending version.