Skip to content

fix(CLI): cache ignored when using --parallel with phrase pull#1085

Open
Thibaut Etienne (tetienne) wants to merge 6 commits intophrase:mainfrom
tetienne:fix/pull-cache-ignored-with-parallel
Open

fix(CLI): cache ignored when using --parallel with phrase pull#1085
Thibaut Etienne (tetienne) wants to merge 6 commits intophrase:mainfrom
tetienne:fix/pull-cache-ignored-with-parallel

Conversation

@tetienne
Copy link
Copy Markdown
Contributor

@tetienne Thibaut Etienne (tetienne) commented Apr 3, 2026

When splitting the work between #1066 (--cache) and #1067 (--parallel), I forgot to thread the cache through the parallel code path. As a result, --cache --parallel silently ignored the cache: no conditional headers were sent, no ETags were stored, and every run re-downloaded all locale files.

Tested against a production project with 204 locale files:

# First run — populates cache
$ ./phrase pull --cache --parallel
Downloaded en to locales/en/tradfile.json
...
(204 files downloaded)

# Second run — all 304s, nothing re-downloaded
$ ./phrase pull --cache --parallel
Not modified locales/en/tradfile.json
...
(204 files skipped)

Test plan

  • go build compiles
  • Manual test: phrase pull --cache --parallel second run returns 304 for all files
  • phrase pull --parallel without --cache behaves identically to before
  • phrase pull --cache without --parallel unaffected

@tetienne Thibaut Etienne (tetienne) marked this pull request as draft April 3, 2026 06:29
Restore ctx.Err() check in PullParallel goroutines to respect
context cancellation after first failure. Make copyToDestination
fail explicitly on nil file instead of silently creating empty
files. Remove dead 304 check after rate-limit retry. Simplify
pull dispatch by replacing pullFn type with if/else. Add mutex
to Save() and json:"-" to mu field for defensive thread safety.
@tetienne Thibaut Etienne (tetienne) marked this pull request as ready for review April 3, 2026 08:20
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